From d36614f7383caf3a717f913d9f9879e7076aff8e Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 29 Dec 2017 17:05:25 +0000 Subject: [PATCH] Make the functions which link the pf state keys to mbufs, inpcbs, or other states more consistent. OK visa@ sashan@ on a previous version --- sys/kern/uipc_mbuf.c | 8 ++-- sys/net/if.c | 4 +- sys/net/pf.c | 93 +++++++++++++++++++++++++++--------------- sys/net/pfvar.h | 10 ++--- sys/netinet/ip_input.c | 4 +- 5 files changed, 71 insertions(+), 48 deletions(-) diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c index 3265238ac8d..bd89446fa98 100644 --- a/sys/kern/uipc_mbuf.c +++ b/sys/kern/uipc_mbuf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uipc_mbuf.c,v 1.250 2017/10/12 09:14:16 mpi Exp $ */ +/* $OpenBSD: uipc_mbuf.c,v 1.251 2017/12/29 17:05:25 bluhm Exp $ */ /* $NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $ */ /* @@ -307,7 +307,7 @@ m_resethdr(struct mbuf *m) m_tag_delete_chain(m); #if NPF > 0 - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); #endif /* NPF > 0 */ /* like m_inithdr(), but keep any associated data and mbufs */ @@ -407,7 +407,7 @@ m_free(struct mbuf *m) if (m->m_flags & M_PKTHDR) { m_tag_delete_chain(m); #if NPF > 0 - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); #endif /* NPF > 0 */ } if (m->m_flags & M_EXT) @@ -1325,7 +1325,7 @@ m_dup_pkthdr(struct mbuf *to, struct mbuf *from, int wait) to->m_pkthdr = from->m_pkthdr; #if NPF > 0 - pf_pkt_state_key_ref(to); + pf_mbuf_link_state_key(to, from->m_pkthdr.pf.statekey); #endif /* NPF > 0 */ SLIST_INIT(&to->m_pkthdr.ph_tags); diff --git a/sys/net/if.c b/sys/net/if.c index f7a2275d8b5..184867b8d61 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if.c,v 1.531 2017/12/15 01:37:30 dlg Exp $ */ +/* $OpenBSD: if.c,v 1.532 2017/12/29 17:05:25 bluhm Exp $ */ /* $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $ */ /* @@ -697,7 +697,7 @@ if_enqueue(struct ifnet *ifp, struct mbuf *m) #endif #if NPF > 0 - pf_pkt_unlink_state_key(m); + pf_pkt_addr_changed(m); #endif /* NPF > 0 */ /* diff --git a/sys/net/pf.c b/sys/net/pf.c index 274f9608e7e..02ff6bcefdf 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1052 2017/12/28 11:37:44 bluhm Exp $ */ +/* $OpenBSD: pf.c,v 1.1053 2017/12/29 17:05:25 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -245,12 +245,17 @@ int pf_match_rule(struct pf_test_ctx *, void pf_counters_inc(int, struct pf_pdesc *, struct pf_state *, struct pf_rule *, struct pf_rule *); -void pf_state_key_link(struct pf_state_key *, + +int pf_state_key_isvalid(struct pf_state_key *); +struct pf_state_key *pf_state_key_ref(struct pf_state_key *); +void pf_state_key_unref(struct pf_state_key *); +void pf_state_key_link_reverse(struct pf_state_key *, struct pf_state_key *); -void pf_inpcb_unlink_state_key(struct inpcb *); void pf_state_key_unlink_reverse(struct pf_state_key *); void pf_state_key_link_inpcb(struct pf_state_key *, struct inpcb *); +void pf_state_key_unlink_inpcb(struct pf_state_key *); +void pf_inpcb_unlink_state_key(struct inpcb *); #if NPFLOG > 0 void pf_log_matches(struct pf_pdesc *, struct pf_rule *, @@ -805,8 +810,7 @@ pf_state_key_detach(struct pf_state *s, int idx) RB_REMOVE(pf_state_tree, &pf_statetbl, sk); sk->removed = 1; pf_state_key_unlink_reverse(sk); - pf_inpcb_unlink_state_key(sk->inp); - sk->inp = NULL; + pf_state_key_unlink_inpcb(sk); pf_state_key_unref(sk); } } @@ -1060,7 +1064,7 @@ pf_find_state(struct pfi_kif *kif, struct pf_state_key_cmp *key, u_int dir, pkt_sk = m->m_pkthdr.pf.statekey; if (!pf_state_key_isvalid(pkt_sk)) { - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); pkt_sk = NULL; } @@ -1086,7 +1090,7 @@ pf_find_state(struct pfi_kif *kif, struct pf_state_key_cmp *key, u_int dir, return (NULL); if (dir == PF_OUT && pkt_sk && pf_compare_state_keys(pkt_sk, sk, kif, dir) == 0) - pf_state_key_link(sk, pkt_sk); + pf_state_key_link_reverse(sk, pkt_sk); else if (dir == PF_OUT && m->m_pkthdr.pf.inp && !m->m_pkthdr.pf.inp->inp_pf_sk && !sk->inp) pf_state_key_link_inpcb(sk, m->m_pkthdr.pf.inp); @@ -7143,7 +7147,7 @@ pf_ouraddr(struct mbuf *m) void pf_pkt_addr_changed(struct mbuf *m) { - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); m->m_pkthdr.pf.inp = NULL; } @@ -7154,7 +7158,7 @@ pf_inp_lookup(struct mbuf *m) struct pf_state_key *sk = m->m_pkthdr.pf.statekey; if (!pf_state_key_isvalid(sk)) - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); else inp = m->m_pkthdr.pf.statekey->inp; @@ -7170,7 +7174,7 @@ pf_inp_link(struct mbuf *m, struct inpcb *inp) struct pf_state_key *sk = m->m_pkthdr.pf.statekey; if (!pf_state_key_isvalid(sk)) { - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); return; } @@ -7179,32 +7183,29 @@ pf_inp_link(struct mbuf *m, struct inpcb *inp) * state, which might be just being marked as deleted by another * thread. */ - if (inp && !sk->inp && !inp->inp_pf_sk) { - sk->inp = inp; - inp->inp_pf_sk = pf_state_key_ref(sk); - } + if (inp && !sk->inp && !inp->inp_pf_sk) + pf_state_key_link_inpcb(sk, inp); + /* The statekey has finished finding the inp, it is no longer needed. */ - pf_pkt_unlink_state_key(m); + pf_mbuf_unlink_state_key(m); } void pf_inp_unlink(struct inpcb *inp) { - if (inp->inp_pf_sk) { - inp->inp_pf_sk->inp = NULL; - pf_inpcb_unlink_state_key(inp); - } + pf_inpcb_unlink_state_key(inp); } void -pf_state_key_link(struct pf_state_key *sk, struct pf_state_key *pkt_sk) +pf_state_key_link_reverse(struct pf_state_key *sk, struct pf_state_key *skrev) { /* * Assert will not wire as long as we are called by pf_find_state() */ - KASSERT((pkt_sk->reverse == NULL) && (sk->reverse == NULL)); - pkt_sk->reverse = pf_state_key_ref(sk); - sk->reverse = pf_state_key_ref(pkt_sk); + KASSERT(sk->reverse == NULL); + sk->reverse = pf_state_key_ref(skrev); + KASSERT(skrev->reverse == NULL); + skrev->reverse = pf_state_key_ref(sk); } #if NPFLOG > 0 @@ -7236,7 +7237,7 @@ pf_state_key_ref(struct pf_state_key *sk) void pf_state_key_unref(struct pf_state_key *sk) { - if ((sk != NULL) && PF_REF_RELE(sk->refcnt)) { + if (PF_REF_RELE(sk->refcnt)) { /* state key must be removed from tree */ KASSERT(!pf_state_key_isvalid(sk)); /* state key must be unlinked from reverse key */ @@ -7254,16 +7255,20 @@ pf_state_key_isvalid(struct pf_state_key *sk) } void -pf_pkt_unlink_state_key(struct mbuf *m) +pf_mbuf_unlink_state_key(struct mbuf *m) { - pf_state_key_unref(m->m_pkthdr.pf.statekey); - m->m_pkthdr.pf.statekey = NULL; + struct pf_state_key *sk = m->m_pkthdr.pf.statekey; + + if (sk != NULL) { + m->m_pkthdr.pf.statekey = NULL; + pf_state_key_unref(sk); + } } void -pf_pkt_state_key_ref(struct mbuf *m) +pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk) { - pf_state_key_ref(m->m_pkthdr.pf.statekey); + m->m_pkthdr.pf.statekey = pf_state_key_ref(sk); } void @@ -7278,19 +7283,39 @@ pf_state_key_link_inpcb(struct pf_state_key *sk, struct inpcb *inp) void pf_inpcb_unlink_state_key(struct inpcb *inp) { + struct pf_state_key *sk = inp->inp_pf_sk; + + if (sk != NULL) { + KASSERT(sk->inp == inp); + sk->inp = NULL; + inp->inp_pf_sk = NULL; + pf_state_key_unref(sk); + } +} + +void +pf_state_key_unlink_inpcb(struct pf_state_key *sk) +{ + struct inpcb *inp = sk->inp; + if (inp != NULL) { - pf_state_key_unref(inp->inp_pf_sk); + KASSERT(inp->inp_pf_sk == sk); + sk->inp = NULL; inp->inp_pf_sk = NULL; + pf_state_key_unref(sk); } } void pf_state_key_unlink_reverse(struct pf_state_key *sk) { - if ((sk != NULL) && (sk->reverse != NULL)) { - pf_state_key_unref(sk->reverse->reverse); - sk->reverse->reverse = NULL; - pf_state_key_unref(sk->reverse); + struct pf_state_key *skrev = sk->reverse; + + if (skrev != NULL) { + KASSERT(skrev->reverse == sk); sk->reverse = NULL; + skrev->reverse = NULL; + pf_state_key_unref(skrev); + pf_state_key_unref(sk); } } diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 27dcefbe47b..13f7fa9389e 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar.h,v 1.469 2017/11/28 16:05:46 bluhm Exp $ */ +/* $OpenBSD: pfvar.h,v 1.470 2017/12/29 17:05:25 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1905,11 +1905,9 @@ int pf_map_addr(sa_family_t, struct pf_rule *, struct pf_pool *, enum pf_sn_types); int pf_postprocess_addr(struct pf_state *); -struct pf_state_key *pf_state_key_ref(struct pf_state_key *); -void pf_state_key_unref(struct pf_state_key *); -int pf_state_key_isvalid(struct pf_state_key *); -void pf_pkt_unlink_state_key(struct mbuf *); -void pf_pkt_state_key_ref(struct mbuf *); +void pf_mbuf_link_state_key(struct mbuf *, + struct pf_state_key *); +void pf_mbuf_unlink_state_key(struct mbuf *); u_int8_t pf_get_wscale(struct pf_pdesc *); u_int16_t pf_get_mss(struct pf_pdesc *); diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index 8428a893976..b3aa0318c8f 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_input.c,v 1.335 2017/12/04 13:40:34 bluhm Exp $ */ +/* $OpenBSD: ip_input.c,v 1.336 2017/12/29 17:05:25 bluhm Exp $ */ /* $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $ */ /* @@ -1459,7 +1459,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt) m_copydata(m, 0, len, mfake.m_pktdat); mfake.m_pkthdr.len = mfake.m_len = len; #if NPF > 0 - pf_pkt_unlink_state_key(&mfake); + pf_pkt_addr_changed(&mfake); #endif /* NPF > 0 */ fake = 1; } -- 2.20.1