git: f084f28a52c5 - main - pf: fix NULL dereference in pfr_set_addrs() with feedback
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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"
}