Hi,
I missed that pf divert packet is not MP safe yet. The problem is
that divert_packet() is called from pf with shared net lock and
sbappendaddr() needs exclusive net lock.
The direct call from pf in IP layer to divert in protocol layer is
not nice. I not sure how to address that.
As a first step clean up divert_packet():
- the function never returns an error
- call variables so and sin like everywhere else
- use goto bad for error handling
- fix error counter
- introduce mutex and refcounting for inp like in the other pcb
functions
Divert packet is still not MP safe, I will fix it later.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1128
diff -u -p -r1.1128 pf.c
--- net/pf.c 3 May 2022 13:32:47 -0000 1.1128
+++ net/pf.c 4 May 2022 20:16:40 -0000
@@ -7403,13 +7403,13 @@ done:
case PF_DIVERT:
switch (pd.af) {
case AF_INET:
- if (!divert_packet(pd.m, pd.dir, r->divert.port))
- pd.m = NULL;
+ divert_packet(pd.m, pd.dir, r->divert.port);
+ pd.m = NULL;
break;
#ifdef INET6
case AF_INET6:
- if (!divert6_packet(pd.m, pd.dir, r->divert.port))
- pd.m = NULL;
+ divert6_packet(pd.m, pd.dir, r->divert.port);
+ pd.m = NULL;
break;
#endif /* INET6 */
}
Index: netinet/ip_divert.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.66
diff -u -p -r1.66 ip_divert.c
--- netinet/ip_divert.c 25 Feb 2022 23:51:03 -0000 1.66
+++ netinet/ip_divert.c 4 May 2022 18:17:49 -0000
@@ -171,30 +171,37 @@ fail:
return (error ? error : EINVAL);
}
-int
+void
divert_packet(struct mbuf *m, int dir, u_int16_t divert_port)
{
- struct inpcb *inp;
- struct socket *sa = NULL;
- struct sockaddr_in addr;
+ struct inpcb *inp = NULL;
+ struct socket *so;
+ struct sockaddr_in sin;
- inp = NULL;
divstat_inc(divs_ipackets);
if (m->m_len < sizeof(struct ip) &&
(m = m_pullup(m, sizeof(struct ip))) == NULL) {
divstat_inc(divs_errors);
- return (0);
+ goto bad;
}
+ mtx_enter(&divbtable.inpt_mtx);
TAILQ_FOREACH(inp, &divbtable.inpt_queue, inp_queue) {
- if (inp->inp_lport == divert_port)
- break;
+ if (inp->inp_lport != divert_port)
+ continue;
+ in_pcbref(inp);
+ break;
+ }
+ mtx_leave(&divbtable.inpt_mtx);
+ if (inp == NULL) {
+ divstat_inc(divs_noport);
+ goto bad;
}
- memset(&addr, 0, sizeof(addr));
- addr.sin_family = AF_INET;
- addr.sin_len = sizeof(addr);
+ memset(&sin, 0, sizeof(sin));
+ sin.sin_family = AF_INET;
+ sin.sin_len = sizeof(sin);
if (dir == PF_IN) {
struct ifaddr *ifa;
@@ -202,37 +209,34 @@ divert_packet(struct mbuf *m, int dir, u
ifp = if_get(m->m_pkthdr.ph_ifidx);
if (ifp == NULL) {
- m_freem(m);
- return (0);
+ divstat_inc(divs_errors);
+ goto bad;
}
TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
if (ifa->ifa_addr->sa_family != AF_INET)
continue;
- addr.sin_addr.s_addr = satosin(
- ifa->ifa_addr)->sin_addr.s_addr;
+ sin.sin_addr = satosin(ifa->ifa_addr)->sin_addr;
break;
}
if_put(ifp);
}
- if (inp) {
- sa = inp->inp_socket;
- if (sbappendaddr(sa, &sa->so_rcv, sintosa(&addr), m, NULL) ==
0) {
- divstat_inc(divs_fullsock);
- m_freem(m);
- return (0);
- } else {
- KERNEL_LOCK();
- sorwakeup(inp->inp_socket);
- KERNEL_UNLOCK();
- }
- }
-
- if (sa == NULL) {
- divstat_inc(divs_noport);
- m_freem(m);
+ so = inp->inp_socket;
+ if (sbappendaddr(so, &so->so_rcv, sintosa(&sin), m, NULL) == 0) {
+ divstat_inc(divs_fullsock);
+ goto bad;
}
- return (0);
+ KERNEL_LOCK();
+ sorwakeup(inp->inp_socket);
+ KERNEL_UNLOCK();
+
+ in_pcbunref(inp);
+ return;
+
+ bad:
+ if (inp != NULL)
+ in_pcbunref(inp);
+ m_freem(m);
}
/*ARGSUSED*/
Index: netinet/ip_divert.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_divert.h,v
retrieving revision 1.14
diff -u -p -r1.14 ip_divert.h
--- netinet/ip_divert.h 25 Feb 2022 23:51:03 -0000 1.14
+++ netinet/ip_divert.h 4 May 2022 18:15:00 -0000
@@ -66,7 +66,7 @@ divstat_inc(enum divstat_counters c)
extern struct inpcbtable divbtable;
void divert_init(void);
-int divert_packet(struct mbuf *, int, u_int16_t);
+void divert_packet(struct mbuf *, int, u_int16_t);
int divert_sysctl(int *, u_int, void *, size_t *, void *, size_t);
int divert_usrreq(struct socket *,
int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
Index: netinet6/ip6_divert.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.65
diff -u -p -r1.65 ip6_divert.c
--- netinet6/ip6_divert.c 25 Feb 2022 23:51:04 -0000 1.65
+++ netinet6/ip6_divert.c 4 May 2022 20:22:56 -0000
@@ -177,30 +177,37 @@ fail:
return (error ? error : EINVAL);
}
-int
+void
divert6_packet(struct mbuf *m, int dir, u_int16_t divert_port)
{
- struct inpcb *inp;
- struct socket *sa = NULL;
- struct sockaddr_in6 addr;
+ struct inpcb *inp = NULL;
+ struct socket *so;
+ struct sockaddr_in6 sin6;
- inp = NULL;
div6stat_inc(div6s_ipackets);
if (m->m_len < sizeof(struct ip6_hdr) &&
(m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) {
div6stat_inc(div6s_errors);
- return (0);
+ goto bad;
}
+ mtx_enter(&divb6table.inpt_mtx);
TAILQ_FOREACH(inp, &divb6table.inpt_queue, inp_queue) {
- if (inp->inp_lport == divert_port)
- break;
+ if (inp->inp_lport != divert_port)
+ continue;
+ in_pcbref(inp);
+ break;
+ }
+ mtx_leave(&divb6table.inpt_mtx);
+ if (inp == NULL) {
+ div6stat_inc(div6s_noport);
+ goto bad;
}
- memset(&addr, 0, sizeof(addr));
- addr.sin6_family = AF_INET6;
- addr.sin6_len = sizeof(addr);
+ memset(&sin6, 0, sizeof(sin6));
+ sin6.sin6_family = AF_INET6;
+ sin6.sin6_len = sizeof(sin6);
if (dir == PF_IN) {
struct ifaddr *ifa;
@@ -208,36 +215,34 @@ divert6_packet(struct mbuf *m, int dir,
ifp = if_get(m->m_pkthdr.ph_ifidx);
if (ifp == NULL) {
- m_freem(m);
- return (0);
+ div6stat_inc(div6s_errors);
+ goto bad;
}
TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
if (ifa->ifa_addr->sa_family != AF_INET6)
continue;
- addr.sin6_addr = satosin6(ifa->ifa_addr)->sin6_addr;
+ sin6.sin6_addr = satosin6(ifa->ifa_addr)->sin6_addr;
break;
}
if_put(ifp);
}
- if (inp) {
- sa = inp->inp_socket;
- if (sbappendaddr(sa, &sa->so_rcv, sin6tosa(&addr), m, NULL) ==
0) {
- div6stat_inc(div6s_fullsock);
- m_freem(m);
- return (0);
- } else {
- KERNEL_LOCK();
- sorwakeup(inp->inp_socket);
- KERNEL_UNLOCK();
- }
- }
-
- if (sa == NULL) {
- div6stat_inc(div6s_noport);
- m_freem(m);
+ so = inp->inp_socket;
+ if (sbappendaddr(so, &so->so_rcv, sin6tosa(&sin6), m, NULL) == 0) {
+ div6stat_inc(div6s_fullsock);
+ goto bad;
}
- return (0);
+ KERNEL_LOCK();
+ sorwakeup(inp->inp_socket);
+ KERNEL_UNLOCK();
+
+ in_pcbunref(inp);
+ return;
+
+ bad:
+ if (inp != NULL)
+ in_pcbunref(inp);
+ m_freem(m);
}
/*ARGSUSED*/
Index: netinet6/ip6_divert.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_divert.h,v
retrieving revision 1.12
diff -u -p -r1.12 ip6_divert.h
--- netinet6/ip6_divert.h 25 Feb 2022 23:51:04 -0000 1.12
+++ netinet6/ip6_divert.h 4 May 2022 20:15:44 -0000
@@ -66,7 +66,7 @@ div6stat_inc(enum div6stat_counters c)
extern struct inpcbtable divb6table;
void divert6_init(void);
-int divert6_packet(struct mbuf *, int, u_int16_t);
+void divert6_packet(struct mbuf *, int, u_int16_t);
int divert6_sysctl(int *, u_int, void *, size_t *, void *, size_t);
int divert6_usrreq(struct socket *,
int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
