git: 0cd655f71b46 - stable/14 - pf: do not reject rules with colliding hashes

The branch stable/14 has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=0cd655f71b46ada2c353c371e2a2f9f0dac29613

commit 0cd655f71b46ada2c353c371e2a2f9f0dac29613
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2026-04-29 15:04:44 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2026-05-09 13:29:04 +0000

    pf: do not reject rules with colliding hashes
    
    We insert rules in pf_krule_global solely for the benefit of the
    'keepcounters' feature. Failing to insert (beause the rule hash
    collides, or an identical rule already exists) would be worse than
    restoring counts to the wrong rule (or failing to restore them at all).
    
    PR:             282863, 294860, 294859, 294858
    MFC after:      3 days
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D56745
    
    (cherry picked from commit a0e4c65f1814a7a677364dc29bb703f84323d175)
---
 sys/netpfil/pf/pf_ioctl.c     | 24 ++++++++----------------
 tests/sys/netpfil/pf/match.sh | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index e824dfcff453..39d3536fe38e 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -2376,14 +2376,12 @@ pf_ioctl_addrule(struct pf_krule *rule, uint32_t ticket,
 
 	PF_RULES_WUNLOCK();
 	pf_hash_rule(rule);
-	if (RB_INSERT(pf_krule_global, ruleset->rules[rs_num].inactive.tree, rule) != NULL) {
-		PF_RULES_WLOCK();
-		TAILQ_REMOVE(ruleset->rules[rs_num].inactive.ptr, rule, entries);
-		ruleset->rules[rs_num].inactive.rcount--;
-		pf_free_rule(rule);
-		rule = NULL;
-		ERROUT(EEXIST);
-	}
+	/**
+	 * Note: rule hashes may collide. Accept this, because the worst that can
+	 * happen is that we get counter preservation wrong.
+	 * Failing to insert here would be worse.
+	 **/
+	RB_INSERT(pf_krule_global, ruleset->rules[rs_num].inactive.tree, rule);
 	PF_CONFIG_UNLOCK();
 
 	return (0);
@@ -3693,14 +3691,8 @@ DIOCGETRULENV_error:
 			ruleset->rules[rs_num].active.rcount--;
 		} else {
 			pf_hash_rule(newrule);
-			if (RB_INSERT(pf_krule_global,
-			    ruleset->rules[rs_num].active.tree, newrule) != NULL) {
-				pf_free_rule(newrule);
-				PF_RULES_WUNLOCK();
-				PF_CONFIG_UNLOCK();
-				error = EEXIST;
-				break;
-			}
+			RB_INSERT(pf_krule_global,
+			    ruleset->rules[rs_num].active.tree, newrule);
 
 			if (oldrule == NULL)
 				TAILQ_INSERT_TAIL(
diff --git a/tests/sys/netpfil/pf/match.sh b/tests/sys/netpfil/pf/match.sh
index bb088c5bf47c..5a1bfb4d51ea 100644
--- a/tests/sys/netpfil/pf/match.sh
+++ b/tests/sys/netpfil/pf/match.sh
@@ -67,7 +67,43 @@ dummynet_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "duplicate_rules" "cleanup"
+duplicate_rules_head()
+{
+	atf_set descr 'Test identical rules'
+	atf_set require.user root
+}
+
+duplicate_rules_body()
+{
+	pft_init
+
+	epair=$(vnet_mkepair)
+	vnet_mkjail alcatraz ${epair}b
+
+	ifconfig ${epair}a 192.0.2.1/24 up
+	jexec alcatraz ifconfig ${epair}b 192.0.2.2/24 up
+
+	# Sanity check
+	atf_check -s exit:0 -o ignore ping -c 1 192.0.2.2
+
+	jexec alcatraz pfctl -e
+	pft_set_rules alcatraz \
+	  "block" \
+	  "pass tagged FOO" \
+	  "match tag FOO" \
+	  "pass tagged FOO"
+
+	atf_check -s exit:0 -o ignore ping -c 3 192.0.2.2
+}
+
+duplicate_rules_cleanup()
+{
+	pft_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "dummynet"
+	atf_add_test_case "duplicate_rules"
 }