git: f084f28a52c5 - main - pf: fix NULL dereference in pfr_set_addrs() with feedback

From: R. Christian McDonald <rcm_at_FreeBSD.org>
Date: Tue, 29 Sep 2026 00:04:17 UTC
The branch main has been updated by rcm:

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

commit f084f28a52c5fb4557ff0b56e98ca36af3d8eec0
Author:     R. Christian McDonald <rcm@FreeBSD.org>
AuthorDate: 2026-09-28 23:01:08 +0000
Commit:     R. Christian McDonald <rcm@FreeBSD.org>
CommitDate: 2026-09-29 00:02:02 +0000

    pf: fix NULL dereference in pfr_set_addrs() with feedback
    
    Since DIOCRSETADDRS was converted to netlink, pf_handle_table_set_addrs()
    calls pfr_set_addrs() with a NULL size2, as the netlink interface has no
    buffer to return the deleted addresses in. pfr_set_addrs() only checked
    size2 for NULL at the end of the function; with PFR_FLAG_FEEDBACK set it
    dereferenced it unconditionally first. pfctl sets PFR_FLAG_FEEDBACK
    when run with -v, so "pfctl -v -t foo -T replace ..." panicked the
    kernel with a NULL pointer dereference. To reproduce:
    
    pfctl -e
    pfctl -t foo -T add 192.0.2.1
    pfctl -v -t foo -T replace 192.0.2.2
    
    Check size2 for NULL before dereferencing it, as is already done at the
    end of the function. The per-address feedback for added and changed
    addresses is still copied back as before; only the list of deleted
    addresses, which the netlink caller has no room for, is skipped.
    
    While here, compare size2 against NULL explicitly in the second check as
    well, per style(9).
    
    Add a regression test.
    
    Approved by:            kp (mentor)
    Fixes:                  08ed87a4a276 ("pf: convert DIOCRSETADDRS to netlink")
    MFC after:              1 week
    Differential Revision:  https://reviews.freebsd.org/D60096
---
 sys/netpfil/pf/pf_table.c     |  4 ++--
 tests/sys/netpfil/pf/table.sh | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/sys/netpfil/pf/pf_table.c b/sys/netpfil/pf/pf_table.c
index dd670d58359b..d11401b6f5a7 100644
--- a/sys/netpfil/pf/pf_table.c
+++ b/sys/netpfil/pf/pf_table.c
@@ -464,7 +464,7 @@ _skip:
 	}
 	if (flags & PFR_FLAG_DONE)
 		pfr_enqueue_addrs(kt, &delq, &xdel, ENQUEUE_UNMARKED_ONLY);
-	if ((flags & PFR_FLAG_FEEDBACK) && *size2) {
+	if ((flags & PFR_FLAG_FEEDBACK) && size2 != NULL && *size2) {
 		if (*size2 < size+xdel) {
 			*size2 = size+xdel;
 			senderr(0);
@@ -490,7 +490,7 @@ _skip:
 		*ndel = xdel;
 	if (nchange != NULL)
 		*nchange = xchange;
-	if ((flags & PFR_FLAG_FEEDBACK) && size2)
+	if ((flags & PFR_FLAG_FEEDBACK) && size2 != NULL)
 		*size2 = size+xdel;
 	pfr_destroy_ktable(tmpkt, 0);
 	return (0);
diff --git a/tests/sys/netpfil/pf/table.sh b/tests/sys/netpfil/pf/table.sh
index 743bfe08557d..c5c9c45c9d3d 100644
--- a/tests/sys/netpfil/pf/table.sh
+++ b/tests/sys/netpfil/pf/table.sh
@@ -812,6 +812,39 @@ replace_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "replace_verbose" "cleanup"
+replace_verbose_head()
+{
+	atf_set descr 'Test table replace command, asked to be verbose'
+	atf_set require.user root
+}
+
+replace_verbose_body()
+{
+	pft_init
+
+	vnet_mkjail alcatraz
+	jexec alcatraz pfctl -e
+
+	pft_set_rules alcatraz \
+	    "table <foo> { 192.0.2.1, 192.0.2.2 }" \
+	    "pass in from <foo> to any"
+
+	# This used to panic: pfr_set_addrs() dereferenced a NULL size2
+	# when asked for feedback over netlink.
+	atf_check -s exit:0 -e "match:1 addresses added." \
+	    -e "match:1 addresses deleted." \
+	    jexec alcatraz pfctl -v -t foo -T replace 192.0.2.2 192.0.2.3
+	atf_check -s exit:0 -o "match:192.0.2.2" -o "match:192.0.2.3" \
+	    -o "not-match:192.0.2.1" \
+	    jexec alcatraz pfctl -t foo -T show
+}
+
+replace_verbose_cleanup()
+{
+	pft_cleanup
+}
+
 atf_test_case "load" "cleanup"
 load_head()
 {
@@ -902,6 +935,7 @@ atf_init_test_cases()
 	atf_add_test_case "show_recursive"
 	atf_add_test_case "in_anchor"
 	atf_add_test_case "replace"
+	atf_add_test_case "replace_verbose"
 	atf_add_test_case "load"
 	atf_add_test_case "test"
 }