From nobody Tue Feb 3 22:51:54 2026 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4f5JdX1Vzqz6QrnP for ; Tue, 03 Feb 2026 22:52:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R13" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4f5JdX05h6z465S for ; Tue, 03 Feb 2026 22:52:00 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1770159120; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=n1J+lDyLva10P4Oj224hTLzORcFlfTCXjpoUrhijS9Q=; b=ao+iSrkGKNIjrPerwxsFL/DY5Ma7K/YwErmJUSAVHod1TZUydzpkSAdgIWv3dqNHxqC9tp WPOw0ytFe7/tFWVDgzoJpssuIgCowidh1sx0pFTViX0tRK4Aj29+bBaYEWbujhut4CTFcq FCpVG+ScpQwtwi559pyB87xxucx1lNK6TUcYS1qUHj7i0hYXQxll+7PemZMycI+gUqsjQE ME2iHYFfhJPZHqKBimstKX9xEwfMZsZRj/IR0ODjoVeRl2NdujeeQ2t8UwBvw+bigU3vFT wjEGm6GjZI22zj2iop74hSpL4YBPjE1KgI2ONJZBibAO2/Blf0X0IFTdJwPmoQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1770159120; a=rsa-sha256; cv=none; b=VOTbYWR2ac8DlPX33sWI72q9APbkmOo+KBMf7nuBt/axO4NneTyt78Y8Rdw3hIgpjFtbms z48e9nrHwRkF4kuOYFwJGsWUYLi6Q65bwyoyEV76L8C7GUA7KKaPb74vyGae1oOrTTzM/3 EnQiXHFp7f6tmfoxryazgLLpGAjqnMSa+EckFuKYEVkIvvdKvKcuUHg3Z26vVKC48rYw/0 q1C/f/TVyO9zF4Gu9ZzlxoWAnKGFnkOYrxURBu6JH5SPgJBAOUyeESRPB4qDWaFQOpf+V+ ndPScKAgHj+7oxkck3pLJXYJ5FU3E8VUAd5vvGNeNyryUn4AUXOiAdt2Vzk17A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1770159120; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=n1J+lDyLva10P4Oj224hTLzORcFlfTCXjpoUrhijS9Q=; b=hSF+kSqE63OY7aoKJZhfyVlHKSNJZXD6143x+rO6ZT/lEszOyfNXYv3W8AgpREpQ85wYhP E6DIiHbB9rfVSXzlShOeygQvyKlTP7xnQvtKu389jnkSjdvKMTER2ENkLf9NGl+sutR3Gc octN0yiK7JvscfBpFlgmeSjz0ztdPllc3hH/F0WH3LA67ELG6NraCbr2kBjsgQwfbkSf0M CuIJ44R3mFbsHVs0XagZ1zMlKLzHaLG7ZPsnvf8BDYaK3noHLblv0MKLoH0RoZE/5HGsdP HzamOYlHLrmfQFyHbdlwg7Xf7EVr/h92J3bMSXjzKioIvI07+w2Ro5AsD9BE0A== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4f5JdW6dKKzZSf for ; Tue, 03 Feb 2026 22:51:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 18aa3 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Tue, 03 Feb 2026 22:51:54 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: fe9e4eb6f38a - main - pf: fix use of uninitialised variable List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: fe9e4eb6f38ae004efb576bf44aded08852f9e6b Auto-Submitted: auto-generated Date: Tue, 03 Feb 2026 22:51:54 +0000 Message-Id: <69827c0a.18aa3.6d334ffe@gitrepo.freebsd.org> The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=fe9e4eb6f38ae004efb576bf44aded08852f9e6b commit fe9e4eb6f38ae004efb576bf44aded08852f9e6b Author: Kristof Provost AuthorDate: 2026-02-03 12:17:08 +0000 Commit: Kristof Provost CommitDate: 2026-02-03 22:51:28 +0000 pf: fix use of uninitialised variable In pf_match_rule() we attempt to append matching rules to the end of 'match_rules'. We want to preserve the order to make the multiple pflog entries easier to understand. So we keep track of the last added rule item in 'rt'. However, that assumed that 'match_rules' was only ever added to in that one call to pf_match_rules(). This isn't always the case, for example if we have match rules in different anchors. In that case we'd end up using the uninitialised 'rt' variable in the SLIST_INSERT_AFTER call. Instead track the match rules and the last matching rule (to enable easy appending) in the struct pf_test_ctx. This also allows us to reduce the number of arguments for some functions, because we passed a ctx to most functions that needed 'match_rules'. While here also make pf_match_rules() static, because it's only ever used in pf.c Add a test case to exercise the relevant code path. MFC after: 2 weeks Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/net/pfvar.h | 7 +++--- sys/netpfil/pf/pf.c | 41 ++++++++++++++---------------- tests/sys/netpfil/pf/match.sh | 58 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index eb17c4ff5ef0..68616a7de5e4 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1442,6 +1442,8 @@ struct pf_test_ctx { int limiter_drop; u_short reason; struct pf_src_node *sns[PF_SN_MAX]; + struct pf_krule_slist *match_rules; + struct pf_krule_item *last_match_rule; struct pf_krule *nr; struct pf_krule *tr; struct pf_krule **rm; @@ -3128,10 +3130,7 @@ int pf_osfp_match(struct pf_osfp_enlist *, pf_osfp_t); #ifdef _KERNEL void pf_print_host(struct pf_addr *, u_int16_t, sa_family_t); -enum pf_test_status pf_step_into_anchor(struct pf_test_ctx *, struct pf_krule *, - struct pf_krule_slist *match_rules); -enum pf_test_status pf_match_rule(struct pf_test_ctx *, struct pf_kruleset *, - struct pf_krule_slist *); +enum pf_test_status pf_step_into_anchor(struct pf_test_ctx *, struct pf_krule *); void pf_step_into_keth_anchor(struct pf_keth_anchor_stackframe *, int *, struct pf_keth_ruleset **, struct pf_keth_rule **, struct pf_keth_rule **, diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 3ccf1344fd7d..b7c79437584e 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -342,14 +342,14 @@ static int pf_dummynet_route(struct pf_pdesc *, struct ifnet *, const struct sockaddr *, struct mbuf **); static int pf_test_eth_rule(int, struct pfi_kkif *, struct mbuf **); +static enum pf_test_status pf_match_rule(struct pf_test_ctx *, struct pf_kruleset *); static int pf_test_rule(struct pf_krule **, struct pf_kstate **, struct pf_pdesc *, struct pf_krule **, struct pf_kruleset **, u_short *, struct inpcb *, struct pf_krule_slist *); static int pf_create_state(struct pf_krule *, struct pf_test_ctx *, - struct pf_kstate **, u_int16_t, u_int16_t, - struct pf_krule_slist *match_rules); + struct pf_kstate **, u_int16_t, u_int16_t); static int pf_state_key_addr_setup(struct pf_pdesc *, struct pf_state_key_cmp *, int); static int pf_tcp_track_full(struct pf_kstate *, @@ -5108,8 +5108,7 @@ pf_tag_packet(struct pf_pdesc *pd, int tag) } while (0) enum pf_test_status -pf_step_into_anchor(struct pf_test_ctx *ctx, struct pf_krule *r, - struct pf_krule_slist *match_rules) +pf_step_into_anchor(struct pf_test_ctx *ctx, struct pf_krule *r) { enum pf_test_status rv; @@ -5127,7 +5126,7 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct pf_krule *r, struct pf_kanchor *child; rv = PF_TEST_OK; RB_FOREACH(child, pf_kanchor_node, &r->anchor->children) { - rv = pf_match_rule(ctx, &child->ruleset, match_rules); + rv = pf_match_rule(ctx, &child->ruleset); if ((rv == PF_TEST_QUICK) || (rv == PF_TEST_FAIL)) { /* * we either hit a rule with quick action @@ -5138,7 +5137,7 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct pf_krule *r, } } } else { - rv = pf_match_rule(ctx, &r->anchor->ruleset, match_rules); + rv = pf_match_rule(ctx, &r->anchor->ruleset); /* * Unless errors occured, stop iff any rule matched * within quick anchors. @@ -5987,10 +5986,9 @@ pf_rule_apply_nat(struct pf_test_ctx *ctx, struct pf_krule *r) } enum pf_test_status -pf_match_rule(struct pf_test_ctx *ctx, struct pf_kruleset *ruleset, - struct pf_krule_slist *match_rules) +pf_match_rule(struct pf_test_ctx *ctx, struct pf_kruleset *ruleset) { - struct pf_krule_item *ri, *rt; + struct pf_krule_item *ri; struct pf_krule *r; struct pf_krule *save_a; struct pf_kruleset *save_aruleset; @@ -6300,12 +6298,12 @@ pf_match_rule(struct pf_test_ctx *ctx, struct pf_kruleset *ruleset, } ri->r = r; - if (SLIST_EMPTY(match_rules)) { - SLIST_INSERT_HEAD(match_rules, ri, entry); + if (SLIST_EMPTY(ctx->match_rules)) { + SLIST_INSERT_HEAD(ctx->match_rules, ri, entry); } else { - SLIST_INSERT_AFTER(rt, ri, entry); + SLIST_INSERT_AFTER(ctx->last_match_rule, ri, entry); } - rt = ri; + ctx->last_match_rule = ri; pf_rule_to_actions(r, &pd->act); if (r->log) @@ -6337,7 +6335,7 @@ pf_match_rule(struct pf_test_ctx *ctx, struct pf_kruleset *ruleset, ctx->source = sr; } if (pd->act.log & PF_LOG_MATCHES) - pf_log_matches(pd, r, ctx->a, ruleset, match_rules); + pf_log_matches(pd, r, ctx->a, ruleset, ctx->match_rules); if (r->quick) { ctx->test_status = PF_TEST_QUICK; break; @@ -6354,7 +6352,7 @@ pf_match_rule(struct pf_test_ctx *ctx, struct pf_kruleset *ruleset, * Note: we don't need to restore if we are not going * to continue with ruleset evaluation. */ - if (pf_step_into_anchor(ctx, r, match_rules) != PF_TEST_OK) { + if (pf_step_into_anchor(ctx, r) != PF_TEST_OK) { break; } ctx->a = save_a; @@ -6391,6 +6389,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, ctx.rsm = rsm; ctx.th = &pd->hdr.tcp; ctx.reason = *reason; + ctx.match_rules = match_rules; pf_addrcpy(&pd->nsaddr, pd->src, pd->af); pf_addrcpy(&pd->ndaddr, pd->dst, pd->af); @@ -6488,7 +6487,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, ruleset = *ctx.rsm; } else { ruleset = &pf_main_ruleset; - rv = pf_match_rule(&ctx, ruleset, match_rules); + rv = pf_match_rule(&ctx, ruleset); if (rv == PF_TEST_FAIL || ctx.limiter_drop == 1) { REASON_SET(reason, ctx.reason); goto cleanup; @@ -6523,7 +6522,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, PFLOG_PACKET(r->action, ctx.reason, r, ctx.a, ruleset, pd, 1, NULL); } if (pd->act.log & PF_LOG_MATCHES) - pf_log_matches(pd, r, ctx.a, ruleset, match_rules); + pf_log_matches(pd, r, ctx.a, ruleset, ctx.match_rules); if (pd->virtual_proto != PF_VPROTO_FRAGMENT && (r->action == PF_DROP) && ((r->rule_flag & PFRULE_RETURNRST) || @@ -6568,8 +6567,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, (pd->flags & PFDESC_TCP_NORM)))) { bool nat64; - action = pf_create_state(r, &ctx, sm, bproto_sum, bip_sum, - match_rules); + action = pf_create_state(r, &ctx, sm, bproto_sum, bip_sum); ctx.sk = ctx.nk = NULL; if (action != PF_PASS) { pf_udp_mapping_release(ctx.udp_mapping); @@ -6659,8 +6657,7 @@ cleanup: static int pf_create_state(struct pf_krule *r, struct pf_test_ctx *ctx, - struct pf_kstate **sm, u_int16_t bproto_sum, u_int16_t bip_sum, - struct pf_krule_slist *match_rules) + struct pf_kstate **sm, u_int16_t bproto_sum, u_int16_t bip_sum) { struct pf_pdesc *pd = ctx->pd; struct pf_kstate *s = NULL; @@ -6729,7 +6726,7 @@ pf_create_state(struct pf_krule *r, struct pf_test_ctx *ctx, s->rule = r; s->nat_rule = ctx->nr; s->anchor = ctx->a; - s->match_rules = *match_rules; + s->match_rules = *ctx->match_rules; SLIST_INIT(&s->linkage); memcpy(&s->act, &pd->act, sizeof(struct pf_rule_actions)); diff --git a/tests/sys/netpfil/pf/match.sh b/tests/sys/netpfil/pf/match.sh index 58c1e021310a..992e32d9f887 100644 --- a/tests/sys/netpfil/pf/match.sh +++ b/tests/sys/netpfil/pf/match.sh @@ -177,9 +177,67 @@ allow_opts_cleanup() pft_cleanup } +atf_test_case "double_match" "cleanup" +double_match_head() +{ + atf_set descr 'Test two match statements in separate anchors' + atf_set require.user root +} + +double_match_body() +{ + pft_init + + epair_one=$(vnet_mkepair) + epair_two=$(vnet_mkepair) + vnet_mkjail rtr ${epair_one}a ${epair_two}a + vnet_mkjail srv ${epair_two}b + + ifconfig ${epair_one}b 192.0.2.2/24 up + + jexec rtr ifconfig ${epair_one}a 192.0.2.1/24 up + jexec rtr ifconfig ${epair_two}a 198.51.100.1/24 up + jexec rtr sysctl net.inet.ip.forwarding=1 + + jexec srv ifconfig ${epair_two}b 198.51.100.2/24 up + + route add default 192.0.2.1 + + # Sanity check + atf_check -s exit:0 -o ignore \ + ping -c 1 192.0.2.1 + + jexec rtr pfctl -e + pft_set_rules rtr \ + "nat on ${epair_two}a from 192.0.2.0/24 to any -> (${epair_two}a)" \ + "block all" \ + "anchor \"userrules\" all {\n \ + anchor \"one\" all { \n\ + match in tag \"allow\"\n\ + }\n\ + anchor \"two\" all { \n\ + match tag \"allow\"\n\ + }\n\ + }\n" \ + "pass quick tagged \"allow\"" + + atf_check -s exit:0 -o ignore \ + ping -c 1 198.51.100.2 + + jexec rtr pfctl -ss -vv + jexec rtr pfctl -sr -vv -a "*" + jexec rtr pfctl -sr -a "*" +} + +double_match_cleanup() +{ + pft_cleanup +} + atf_init_test_cases() { atf_add_test_case "dummynet" atf_add_test_case "quick" atf_add_test_case "allow_opts" + atf_add_test_case "double_match" }