On Wed, Dec 02, 2015 at 04:48:51PM +0100, Alexander Bluhm wrote:
> 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.
Here is a new version of the diff. This is new:
- rename "chain" to "link" after some bikeshedding in the hackroom
- propper function style
- merge with sashan@'s diff
- no pf_inp_enter() function, manipulate the mbuf directly
- do not move the SS_ISCONNECTED checks into pf
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.954
diff -u -p -r1.954 pf.c
--- net/pf.c 2 Dec 2015 16:00:42 -0000 1.954
+++ net/pf.c 2 Dec 2015 18:32:50 -0000
@@ -6728,6 +6728,40 @@ 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_link(struct mbuf *m, struct inpcb *inp)
+{
+ if (m->m_pkthdr.pf.statekey && inp &&
+ !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;
+}
+
+void
+pf_inp_unlink(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.424
diff -u -p -r1.424 pfvar.h
--- net/pfvar.h 2 Dec 2015 16:00:42 -0000 1.424
+++ net/pfvar.h 2 Dec 2015 18:32:50 -0000
@@ -1754,6 +1754,9 @@ 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_link(struct mbuf *, struct inpcb *);
+void pf_inp_unlink(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.189
diff -u -p -r1.189 in_pcb.c
--- netinet/in_pcb.c 2 Dec 2015 16:00:42 -0000 1.189
+++ netinet/in_pcb.c 2 Dec 2015 17:37:56 -0000
@@ -509,8 +509,7 @@ in_pcbdetach(struct inpcb *inp)
if (inp->inp_pf_sk) {
pf_unlink_divert_state(inp->inp_pf_sk);
/* pf_unlink_divert_state() may have detached the state */
- if (inp->inp_pf_sk)
- inp->inp_pf_sk->inp = NULL;
+ pf_inp_unlink(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 17:38:07 -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,8 @@ 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;
+ if (inp->inp_socket->so_state & SS_ISCONNECTED)
+ pf_inp_link(m, inp);
#endif
#ifdef IPSEC
@@ -1294,10 +1279,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_unlink(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 17:25:24 -0000
@@ -1075,7 +1075,8 @@ send:
m->m_pkthdr.ph_rtableid = tp->t_inpcb->inp_rtableid;
#if NPF > 0
- m->m_pkthdr.pf.inp = tp->t_inpcb;
+ if (so->so_state & SS_ISCONNECTED)
+ m->m_pkthdr.pf.inp = 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 17:37: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,8 @@ 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;
+ if (inp->inp_socket->so_state & SS_ISCONNECTED)
+ pf_inp_link(m, inp);
#endif
#ifdef IPSEC
