From nobody Mon Aug 18 10:07:57 2025 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 4c57gy3Mpzz64w8r; Mon, 18 Aug 2025 10:07:58 +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 "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4c57gy0lbHz3fkL; Mon, 18 Aug 2025 10:07:58 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755511678; 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=PAdPRUT8DWzuE4yaQVNk9eEAc7pTxOT3SKq6v2Rx+Yg=; b=w5j35diayvHLtbFHrISO3/cM1mcR2VihBeaNKnLz5scTVQ7vs0buNJmNdq+h5MpV9tja4+ QbEj2g4oAfL3pbRN2YEoOq4R9NLlmFkDkMaA9BJaXubt0xULySPuLyOJuPUvbKMhsqVsIV 3Zxonv7TZ+SRv4ndsJfJ9XOfdsvllm1PCAui49xdXS5gsfTaUtz/yRK/heaYiCMkpNv7Rk VCglkPq7rlEFJ2sy/jsMPAtOji86CbEv2f39kiTIK4nkIrLriBuNf/HHRgJ3XPnRbt6M4k tFSFzDhnpl3LIGm9GJlZfgi3ofxWnkeJivD6ng5PGuUSEViLtWxZ/N2OzS/y4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755511678; 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=PAdPRUT8DWzuE4yaQVNk9eEAc7pTxOT3SKq6v2Rx+Yg=; b=De9qQQJ7rbVDiaemOGh7PvN0tYReT7n3g5dAmXICWvV8H8r7gmlA+IkB5ej1jaL9SL+Ceu jSZeyceZn5KIecT0l/U4OAFV/aIakvpHUfRyf4W1moDebNoyjiF3UAsjxqQIm0nRIuuWie +juCt1upLab9+2k88ipFwmyR2uYa6I8ZywPBODTquwguZuDd+GO8z2PwY2YJatRRwZid4a bzpr1FErgJdSEdGLTCw76W6uqK76SFEEI/QuylyofPsZ0KIpKgWTdFW69k5TCngS63dHQf 2AwJof+BLX8exQPJfBkSS/4Ea7Y8LgnQoXFbpnN9MCesC2e4F1PPmKE+S8xiWA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1755511678; a=rsa-sha256; cv=none; b=x0TDdzby8QjCeji6sT85FxetkAJvR5hxQDF9MtKWTirkrYjTgHumt+NaoBkXERzCGmoNKP ZRUuYWruMsLWztBlVZxj++q+Q8DpomctjON49ZV7HlKgQaWX3Ox9lUmuzoTjWdgKT9LsVH p76XAK//rD9Y6cZWFCuu4eKO/s8rRzs8BRwvkmR7yerF8gbIQ3QfxAkAWaoEMFh9rX16OH 9Tqv43tFFIwA1AZkY6tyk5wvHaow6e2PpK5qFhLvb4Kl57tmJjythrq9p8fg/WAF6mgOQq oXSosNnYuvyh4BHQMgdJLKDvXjXLgvVatpo4Z5luxOXbp9/19m+p/yCr8PMBoQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4c57gy0GsnzsHK; Mon, 18 Aug 2025 10:07:58 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 57IA7vQA055992; Mon, 18 Aug 2025 10:07:57 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 57IA7vRS055989; Mon, 18 Aug 2025 10:07:57 GMT (envelope-from git) Date: Mon, 18 Aug 2025 10:07:57 GMT Message-Id: <202508181007.57IA7vRS055989@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 094a60281b9e - main - pf: fix potential infinite loop adding/deleting addresses in tables 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: 094a60281b9e8faf8f3e1a4497ab46955b5a1417 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=094a60281b9e8faf8f3e1a4497ab46955b5a1417 commit 094a60281b9e8faf8f3e1a4497ab46955b5a1417 Author: Kristof Provost AuthorDate: 2025-08-18 06:42:26 +0000 Commit: Kristof Provost 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()