Hi, To avoid that the stack manipules the pf statekeys directly, introduce some pf_inp_...() functions as an interface. Locks can be added to them later.
I have removed the first chaining at the beginning of tcp_input() and udp_input() directly after in_pcbhashlookup() as it is not necessary. It will be done later anyway. That code was a relict, from the time before I had added the second chaining. sashan@ is working at the pf_unlink_state() loop in in_pcbdetach() so I have not moved that yet. ok? bluhm Index: net/pf.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v retrieving revision 1.952 diff -u -p -r1.952 pf.c --- net/pf.c 21 Nov 2015 11:29:40 -0000 1.952 +++ net/pf.c 2 Dec 2015 15:25:29 -0000 @@ -6714,6 +6714,44 @@ pf_pkt_addr_changed(struct mbuf *m) m->m_pkthdr.pf.inp = NULL; } +struct inpcb * +pf_inp_lookup(struct mbuf *m) { + struct inpcb *inp = NULL; + + if (m->m_pkthdr.pf.statekey) { + inp = m->m_pkthdr.pf.statekey->inp; + if (inp && inp->inp_pf_sk) + KASSERT(m->m_pkthdr.pf.statekey == inp->inp_pf_sk); + } + return (inp); +} + +void +pf_inp_enter(struct mbuf *m, struct inpcb *inp) { + if (inp->inp_socket->so_state & SS_ISCONNECTED) + m->m_pkthdr.pf.inp = inp; +} + +void +pf_inp_chain(struct mbuf *m, struct inpcb *inp) { + if (m->m_pkthdr.pf.statekey && inp && + !m->m_pkthdr.pf.statekey->inp && !inp->inp_pf_sk && + (inp->inp_socket->so_state & SS_ISCONNECTED)) { + m->m_pkthdr.pf.statekey->inp = inp; + inp->inp_pf_sk = m->m_pkthdr.pf.statekey; + } + /* The statekey has finished finding the inp, it is no longer needed. */ + m->m_pkthdr.pf.statekey = NULL; +} + +void +pf_inp_unchain(struct inpcb *inp) { + if (inp->inp_pf_sk) { + inp->inp_pf_sk->inp = NULL; + inp->inp_pf_sk = NULL; + } +} + #if NPFLOG > 0 void pf_log_matches(struct pf_pdesc *pd, struct pf_rule *rm, struct pf_rule *am, Index: net/pfvar.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v retrieving revision 1.423 diff -u -p -r1.423 pfvar.h --- net/pfvar.h 20 Nov 2015 03:35:23 -0000 1.423 +++ net/pfvar.h 2 Dec 2015 15:35:58 -0000 @@ -1753,6 +1753,10 @@ int pf_rtlabel_match(struct pf_addr *, s int pf_socket_lookup(struct pf_pdesc *); struct pf_state_key *pf_alloc_state_key(int); void pf_pkt_addr_changed(struct mbuf *); +struct inpcb *pf_inp_lookup(struct mbuf *); +void pf_inp_enter(struct mbuf *, struct inpcb *); +void pf_inp_chain(struct mbuf *, struct inpcb *); +void pf_inp_unchain(struct inpcb *); int pf_state_key_attach(struct pf_state_key *, struct pf_state *, int); int pf_translate(struct pf_pdesc *, struct pf_addr *, u_int16_t, struct pf_addr *, u_int16_t, u_int16_t, int); Index: netinet/in_pcb.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v retrieving revision 1.188 diff -u -p -r1.188 in_pcb.c --- netinet/in_pcb.c 30 Oct 2015 09:39:42 -0000 1.188 +++ netinet/in_pcb.c 2 Dec 2015 15:30:25 -0000 @@ -518,8 +518,7 @@ in_pcbdetach(struct inpcb *inp) break; } /* pf_unlink_state() may have detached the state */ - if (inp->inp_pf_sk) - inp->inp_pf_sk->inp = NULL; + pf_inp_unchain(inp); } #endif s = splnet(); Index: netinet/tcp_input.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v retrieving revision 1.310 diff -u -p -r1.310 tcp_input.c --- netinet/tcp_input.c 29 Nov 2015 15:09:32 -0000 1.310 +++ netinet/tcp_input.c 2 Dec 2015 15:25:40 -0000 @@ -580,11 +580,7 @@ tcp_input(struct mbuf *m, ...) * Locate pcb for segment. */ #if NPF > 0 - if (m->m_pkthdr.pf.statekey) { - inp = m->m_pkthdr.pf.statekey->inp; - if (inp && inp->inp_pf_sk) - KASSERT(m->m_pkthdr.pf.statekey == inp->inp_pf_sk); - } + inp = pf_inp_lookup(m); #endif findpcb: if (inp == NULL) { @@ -602,12 +598,6 @@ findpcb: m->m_pkthdr.ph_rtableid); break; } -#if NPF > 0 - if (m->m_pkthdr.pf.statekey && inp) { - m->m_pkthdr.pf.statekey->inp = inp; - inp->inp_pf_sk = m->m_pkthdr.pf.statekey; - } -#endif } if (inp == NULL) { int inpl_reverse = 0; @@ -880,13 +870,7 @@ findpcb: #endif #if NPF > 0 - if (m->m_pkthdr.pf.statekey && !m->m_pkthdr.pf.statekey->inp && - !inp->inp_pf_sk) { - m->m_pkthdr.pf.statekey->inp = inp; - inp->inp_pf_sk = m->m_pkthdr.pf.statekey; - } - /* The statekey has finished finding the inp, it is no longer needed. */ - m->m_pkthdr.pf.statekey = NULL; + pf_inp_chain(m, inp); #endif #ifdef IPSEC @@ -1294,10 +1278,7 @@ trimthenstep6: * has already been linked to the socket. Remove the * link between old socket and new state. */ - if (inp->inp_pf_sk) { - inp->inp_pf_sk->inp = NULL; - inp->inp_pf_sk = NULL; - } + pf_inp_unchain(inp); #endif /* * Advance the iss by at least 32768, but Index: netinet/tcp_output.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v retrieving revision 1.115 diff -u -p -r1.115 tcp_output.c --- netinet/tcp_output.c 24 Oct 2015 16:08:48 -0000 1.115 +++ netinet/tcp_output.c 2 Dec 2015 15:20:19 -0000 @@ -78,6 +78,7 @@ #include <sys/socketvar.h> #include <sys/kernel.h> +#include <net/if.h> #include <net/route.h> #include <netinet/in.h> @@ -93,6 +94,10 @@ #include <netinet/tcpip.h> #include <netinet/tcp_debug.h> +#if NPF > 0 +#include <net/pfvar.h> +#endif + #ifdef notyet extern struct mbuf *m_copypack(); #endif @@ -1075,7 +1080,7 @@ send: m->m_pkthdr.ph_rtableid = tp->t_inpcb->inp_rtableid; #if NPF > 0 - m->m_pkthdr.pf.inp = tp->t_inpcb; + pf_inp_enter(m, tp->t_inpcb); #endif switch (tp->pf) { Index: netinet/udp_usrreq.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.207 diff -u -p -r1.207 udp_usrreq.c --- netinet/udp_usrreq.c 11 Sep 2015 07:42:35 -0000 1.207 +++ netinet/udp_usrreq.c 2 Dec 2015 15:23:51 -0000 @@ -527,12 +527,8 @@ udp_input(struct mbuf *m, ...) /* * Locate pcb for datagram. */ -#if 0 - if (m->m_pkthdr.pf.statekey) { - inp = m->m_pkthdr.pf.statekey->inp; - if (inp && inp->inp_pf_sk) - KASSERT(m->m_pkthdr.pf.statekey == inp->inp_pf_sk); - } +#if NPF > 0 && 0 /* currently disabled */ + inp = pf_inp_lookup(); #endif if (inp == NULL) { #ifdef INET6 @@ -544,12 +540,6 @@ udp_input(struct mbuf *m, ...) #endif /* INET6 */ inp = in_pcbhashlookup(&udbtable, ip->ip_src, uh->uh_sport, ip->ip_dst, uh->uh_dport, m->m_pkthdr.ph_rtableid); -#if NPF > 0 - if (m->m_pkthdr.pf.statekey && inp) { - m->m_pkthdr.pf.statekey->inp = inp; - inp->inp_pf_sk = m->m_pkthdr.pf.statekey; - } -#endif } if (inp == 0) { int inpl_reverse = 0; @@ -591,13 +581,7 @@ udp_input(struct mbuf *m, ...) KASSERT(sotoinpcb(inp->inp_socket) == inp); #if NPF > 0 - if (m->m_pkthdr.pf.statekey && !m->m_pkthdr.pf.statekey->inp && - !inp->inp_pf_sk && (inp->inp_socket->so_state & SS_ISCONNECTED)) { - m->m_pkthdr.pf.statekey->inp = inp; - inp->inp_pf_sk = m->m_pkthdr.pf.statekey; - } - /* The statekey has finished finding the inp, it is no longer needed. */ - m->m_pkthdr.pf.statekey = NULL; + pf_inp_chain(m, inp); #endif #ifdef IPSEC @@ -1036,8 +1020,7 @@ udp_output(struct inpcb *inp, struct mbu m->m_pkthdr.ph_rtableid = inp->inp_rtableid; #if NPF > 0 - if (inp->inp_socket->so_state & SS_ISCONNECTED) - m->m_pkthdr.pf.inp = inp; + pf_inp_enter(m, inp); #endif error = ip_output(m, inp->inp_options, &inp->inp_route,