The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=094a60281b9e8faf8f3e1a4497ab46955b5a1417
commit 094a60281b9e8faf8f3e1a4497ab46955b5a1417
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-08-18 06:42:26 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-08-18 10:03:23 +0000
pf: fix potential infinite loop adding/deleting addresses in tables
The 'nadd' returned by these calls is the number of addresses actually added
or deleted. It can differ from the number userspace sent to the kernel if
the
addresses are already present (or not present for the delete case).
This meant that if all of the addresses were already handled the kernel
would
return zero, putting us in an infinite loop.
Handle this, and extend the test case to provoke this scenario.
Reported by: netchild@
Fixes: bad279e12deb ("pf: convert DIOCRDELADDRS to netlink")
Fixes: 8b388995b8b2 ("pf: convert DIOCRADDADDRS to netlink")
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
lib/libpfctl/libpfctl.c | 22 ++++++++++------------
tests/sys/netpfil/pf/table.sh | 24 +++++++++++++++++++++++-
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c
index cbd9d4677146..0037f31df04b 100644
--- a/lib/libpfctl/libpfctl.c
+++ b/lib/libpfctl/libpfctl.c
@@ -2453,7 +2453,7 @@ _pfctl_table_add_addrs_h(struct pfctl_handle *h, struct
pfr_table *tbl, struct p
snl_add_msg_attr_table(&nw, PF_TA_TABLE, tbl);
snl_add_msg_attr_u32(&nw, PF_TA_FLAGS, flags);
- for (int i = 0; i < size && i < 256; i++)
+ for (int i = 0; i < size; i++)
snl_add_msg_attr_pfr_addr(&nw, PF_TA_ADDR, &addrs[i]);
if ((hdr = snl_finalize_msg(&nw)) == NULL)
@@ -2481,19 +2481,18 @@ pfctl_table_add_addrs_h(struct pfctl_handle *h, struct
pfr_table *tbl, struct pf
int ret;
int off = 0;
int partial_added;
+ int chunk_size;
do {
- ret = _pfctl_table_add_addrs_h(h, tbl, &addr[off], size - off,
&partial_added, flags);
+ chunk_size = MIN(size - off, 256);
+ ret = _pfctl_table_add_addrs_h(h, tbl, &addr[off], chunk_size,
&partial_added, flags);
if (ret != 0)
break;
if (nadd)
*nadd += partial_added;
- off += partial_added;
+ off += chunk_size;
} while (off < size);
- if (nadd)
- *nadd = off;
-
return (ret);
}
@@ -2521,7 +2520,7 @@ _pfctl_table_del_addrs_h(struct pfctl_handle *h, struct
pfr_table *tbl, struct p
snl_add_msg_attr_table(&nw, PF_TA_TABLE, tbl);
snl_add_msg_attr_u32(&nw, PF_TA_FLAGS, flags);
- for (int i = 0; i < size && i < 256; i++)
+ for (int i = 0; i < size; i++)
snl_add_msg_attr_pfr_addr(&nw, PF_TA_ADDR, &addrs[i]);
if ((hdr = snl_finalize_msg(&nw)) == NULL)
@@ -2572,20 +2571,19 @@ pfctl_table_del_addrs_h(struct pfctl_handle *h, struct
pfr_table *tbl, struct pf
int ret;
int off = 0;
int partial_deleted;
+ int chunk_size;
do {
- ret = _pfctl_table_del_addrs_h(h, tbl, &addr[off], size - off,
+ chunk_size = MIN(size - off, 256);
+ ret = _pfctl_table_del_addrs_h(h, tbl, &addr[off], chunk_size,
&partial_deleted, flags);
if (ret != 0)
break;
if (ndel)
*ndel += partial_deleted;
- off += partial_deleted;
+ off += chunk_size;
} while (off < size);
- if (ndel)
- *ndel = off;
-
return (ret);
}
diff --git a/tests/sys/netpfil/pf/table.sh b/tests/sys/netpfil/pf/table.sh
index c773518e95e4..65492545a13b 100644
--- a/tests/sys/netpfil/pf/table.sh
+++ b/tests/sys/netpfil/pf/table.sh
@@ -641,9 +641,31 @@ large_body()
-e match:"${expected}/${expected} addresses added." \
jexec alcatraz pfctl -t foo -T add -f ${pwd}/foo.lst
actual=$(jexec alcatraz pfctl -t foo -T show | wc -l | awk '{ print $1;
}')
- if [[ $actual -ne $expected ]]; then
+ if [ $actual -ne $expected ]; then
atf_fail "Unexpected number of table entries $expected $acual"
fi
+
+ # The second pass should work too, but confirm we've inserted everything
+ atf_check -s exit:0 \
+ -e match:"0/${expected} addresses added." \
+ jexec alcatraz pfctl -t foo -T add -f ${pwd}/foo.lst
+
+ echo '42.42.42.42' >> ${pwd}/foo.lst
+ expected=$((${expected} + 1))
+
+ # And we can also insert one additional address
+ atf_check -s exit:0 \
+ -e match:"1/${expected} addresses added." \
+ jexec alcatraz pfctl -t foo -T add -f ${pwd}/foo.lst
+
+ # Try to delete one address
+ atf_check -s exit:0 \
+ -e match:"1/1 addresses deleted." \
+ jexec alcatraz pfctl -t foo -T delete 42.42.42.42
+ # And again, for the same address
+ atf_check -s exit:0 \
+ -e match:"0/1 addresses deleted." \
+ jexec alcatraz pfctl -t foo -T delete 42.42.42.42
}
large_cleanup()
