Date: Tue, 15 Apr 2025 12:46:34 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 9c68e37d96b9 - main - pf: share reason between pf_test() and pf_test_rule() Message-ID: <202504151246.53FCkYSt078263@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=9c68e37d96b932612f2819232c8a934eaeebb557 commit 9c68e37d96b932612f2819232c8a934eaeebb557 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-04-10 08:33:07 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-04-15 07:47:47 +0000 pf: share reason between pf_test() and pf_test_rule() pass a pointer to pf_test()'s reason to pf_test_rule instead of using a local one. While we always intended to keep the logging in pf_test_rule and pf_test so seperate that we don't end up with a wrong reason, this is just too fragile and I can't even convince myself that it still is right. pointed out by markus, ok bluhm benno Obtained from: OpenBSD, henning <henning@openbsd.org>, f25274e4c5 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/pf/pf.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index fdd412a92135..9d965d583629 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -342,7 +342,7 @@ static int pf_test_eth_rule(int, struct pfi_kkif *, struct mbuf **); static int pf_test_rule(struct pf_krule **, struct pf_kstate **, struct pf_pdesc *, struct pf_krule **, - struct pf_kruleset **, struct inpcb *); + struct pf_kruleset **, u_short *, struct inpcb *); static int pf_create_state(struct pf_krule *, struct pf_krule *, struct pf_krule *, struct pf_pdesc *, struct pf_state_key *, struct pf_state_key *, int *, @@ -5478,7 +5478,7 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0) static int pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pf_pdesc *pd, struct pf_krule **am, - struct pf_kruleset **rsm, struct inpcb *inp) + struct pf_kruleset **rsm, u_short *reason, struct inpcb *inp) { struct pf_krule *nr = NULL; struct pf_krule *r, *a = NULL; @@ -5487,7 +5487,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pf_krule_item *ri; struct tcphdr *th = &pd->hdr.tcp; struct pf_state_key *sk = NULL, *nk = NULL; - u_short reason, transerror; + u_short transerror; int rewrite = 0; int tag = -1; int asd = 0; @@ -5575,7 +5575,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, switch (transerror) { default: /* A translation error occurred. */ - REASON_SET(&reason, transerror); + REASON_SET(reason, transerror); goto cleanup; case PFRES_MAX: /* No match. */ @@ -5859,7 +5859,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, if (r->action == PF_MATCH) { ri = malloc(sizeof(struct pf_krule_item), M_PF_RULE_ITEM, M_NOWAIT | M_ZERO); if (ri == NULL) { - REASON_SET(&reason, PFRES_MEMORY); + REASON_SET(reason, PFRES_MEMORY); goto cleanup; } ri->r = r; @@ -5873,7 +5873,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, pd->naf = r->naf; if (pd->af != pd->naf) { if (pf_get_transaddr_af(r, pd) == -1) { - REASON_SET(&reason, PFRES_TRANSLATE); + REASON_SET(reason, PFRES_TRANSLATE); goto cleanup; } } @@ -5903,7 +5903,7 @@ nextrule: a = *am; ruleset = *rsm; - REASON_SET(&reason, PFRES_MATCH); + REASON_SET(reason, PFRES_MATCH); /* apply actions for last matching pass/block rule */ pf_rule_to_actions(r, &pd->act); @@ -5911,7 +5911,7 @@ nextrule: pd->naf = r->naf; if (pd->af != pd->naf) { if (pf_get_transaddr_af(r, pd) == -1) { - REASON_SET(&reason, PFRES_TRANSLATE); + REASON_SET(reason, PFRES_TRANSLATE); goto cleanup; } } @@ -5919,7 +5919,7 @@ nextrule: if (r->log) { if (rewrite) m_copyback(pd->m, pd->off, pd->hdrlen, pd->hdr.any); - PFLOG_PACKET(r->action, reason, r, a, ruleset, pd, 1, NULL); + PFLOG_PACKET(r->action, *reason, r, a, ruleset, pd, 1, NULL); } if (pd->act.log & PF_LOG_MATCHES) pf_log_matches(pd, r, a, ruleset, &match_rules); @@ -5929,14 +5929,14 @@ nextrule: (r->rule_flag & PFRULE_RETURNICMP) || (r->rule_flag & PFRULE_RETURN))) { pf_return(r, nr, pd, th, bproto_sum, - bip_sum, &reason, r->rtableid); + bip_sum, reason, r->rtableid); } if (r->action == PF_DROP) goto cleanup; if (tag > 0 && pf_tag_packet(pd, tag)) { - REASON_SET(&reason, PFRES_MEMORY); + REASON_SET(reason, PFRES_MEMORY); goto cleanup; } if (pd->act.rtableid >= 0) @@ -5957,9 +5957,9 @@ nextrule: */ pd->act.rt = r->rt; /* Don't use REASON_SET, pf_map_addr increases the reason counters */ - reason = pf_map_addr_sn(pd->af, r, pd->src, &pd->act.rt_addr, + *reason = pf_map_addr_sn(pd->af, r, pd->src, &pd->act.rt_addr, &pd->act.rt_kif, NULL, &sn, &snh, pool, PF_SN_ROUTE); - if (reason != 0) + if (*reason != 0) goto cleanup; } @@ -5978,7 +5978,7 @@ nextrule: if (action == PF_DROP && (r->rule_flag & PFRULE_RETURN)) pf_return(r, nr, pd, th, - bproto_sum, bip_sum, &reason, + bproto_sum, bip_sum, reason, pd->act.rtableid); return (action); } @@ -7279,6 +7279,7 @@ pf_sctp_multihome_delayed(struct pf_pdesc *pd, struct pfi_kkif *kif, struct pf_krule *ra = NULL; struct pf_krule *r = &V_pf_default_rule; struct pf_kruleset *rs = NULL; + u_short reason; bool do_extra = true; PF_RULES_RLOCK_TRACKER; @@ -7319,7 +7320,7 @@ again: j->pd.related_rule = s->rule; } ret = pf_test_rule(&r, &sm, - &j->pd, &ra, &rs, NULL); + &j->pd, &ra, &rs, &reason, NULL); PF_RULES_RUNLOCK(); SDT_PROBE4(pf, sctp, multihome, test, kif, r, j->pd.m, ret); if (ret != PF_DROP && sm != NULL) { @@ -10367,7 +10368,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 action = PF_DROP; else action = pf_test_rule(&r, &s, &pd, &a, - &ruleset, inp); + &ruleset, &reason, inp); if (action != PF_PASS) REASON_SET(&reason, PFRES_FRAG); break; @@ -10425,7 +10426,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 break; } else { action = pf_test_rule(&r, &s, &pd, - &a, &ruleset, inp); + &a, &ruleset, &reason, inp); } } break; @@ -10446,7 +10447,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 a = s->anchor; } else if (s == NULL) { action = pf_test_rule(&r, &s, - &pd, &a, &ruleset, inp); + &pd, &a, &ruleset, &reason, inp); } break; @@ -10474,7 +10475,7 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0 a = s->anchor; } else if (s == NULL) action = pf_test_rule(&r, &s, &pd, - &a, &ruleset, inp); + &a, &ruleset, &reason, inp); break; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202504151246.53FCkYSt078263>