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,

Reply via email to