From nobody Wed Oct 28 16:56:59 2026 X-Original-To: dev-commits-src-all@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 4jFD6m498kz6wmB9 for ; Wed, 28 Oct 2026 16:57:04 +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) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4jFD6m2QRLz3F3Y for ; Wed, 28 Oct 2026 16:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793206624; 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=AmhicoUfh21P+qybcjLqC2yes8YSEL4xbmz30riWeN4=; b=NSds8TE7t/UEy1+EO+jf5tJk0MU1tLt1SFKyDMiVyqVMZcuuaACr8DZClmSDeRxPn05LhP 2VccZcAfejS59IT0bpu+4SRikM8T3iewiq5uBbzRCPXr3PDCoFk5gFNULWAoEUEadfkr6u UUGItCApXU3nXh0EGc/GoOfLkfwxFWDa8fYYr24DflPYnKuWp2jQ+h78RwHWDCmO9iQhzh SwLlkQs8bsdnB+f1VKsBbvZb05eiYpnk4U6yT8EobELeqIBDblOyJQTeVx5mu15CVo9Gfr g7ci6FNnYh4s7td1dvbfdugjmpmE9d56WprJrZ9Z/uaCVJwh/VtcgI880NAy3A== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1793206624; b=i/Tq2AEK8JkKCUMwWsd1IDXoYtTPmCIvmquE8T69/cDoc1LkZfdo0f3KIZajz22Lk0u+Ox 13e1d+W2onuwSTtMfdBACyu3fu+8caHnlLIthDe3ObN+rNYXtLvMXTvJJ4Fhpm7MuGkXtG zY8SEWJtopgMTu2t2kc0pxkFOqW2NqLoNe4jgXJf++koPMpbdW/9PYXm63nJXui/3avvBv fo8wiSIO+YY+1VAzVE6vdt/KBmTAL2RnmqmqsOceJ47HWkuPBIixlIpNl+C5QA1KaTwbhY Smdu68ge2n3/wdgveN8fW3tcK6phaE+joUCsbRY5mjguQSZQ9OK39vI1FVHTHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793206624; 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=AmhicoUfh21P+qybcjLqC2yes8YSEL4xbmz30riWeN4=; b=nfamijxwaqelCQ5pXHCR4i/0+H3olip7dVgD6HMbOffvrMNxbk631Tupn1CqrZRQ33ORig iHjm+K62NwVrLiucF25nhUpGQ+/qMlUvx1FeP5HlD8jkDj5ePeVDxLl8IHUNyhnQvxNZOB 4+He1hEjnTZ4O+qtV9pZUbQT/GmuxvTPUbQBM51GrAYX6ZFuy8GzMjcN5mo+vYMUVLALCM 53sCiZLco/Q6Se0wNQMVAETR1AbWlo40/+zQD0d/Jjufg7sy9YPF6n/DkvRmVQCp1OCmTT PONtsU7S7rwvr7tSZ7Mzjo1E0tlr/26y9y2C8KfnAq1uTI7TOFNad602y1SBGg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4jFD6m24v7z1G40 for ; Wed, 28 Oct 2026 16:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 32ee4 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Wed, 28 Oct 2026 16:56:59 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: R. Christian McDonald Subject: git: 389ae8e4fabe - main - pf: Free the stack key when attaching it collides List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: Commit messages for all branches of the src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rcm X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 389ae8e4fabeaf1c655f8116b1a1402e597fa515 Auto-Submitted: auto-generated Date: Wed, 28 Oct 2026 16:56:59 +0000 Message-Id: <6ae2295b.32ee4.4ab048eb@gitrepo.freebsd.org> The branch main has been updated by rcm: URL: https://cgit.FreeBSD.org/src/commit/?id=389ae8e4fabeaf1c655f8116b1a1402e597fa515 commit 389ae8e4fabeaf1c655f8116b1a1402e597fa515 Author: R. Christian McDonald AuthorDate: 2026-10-28 16:55:07 +0000 Commit: R. Christian McDonald CommitDate: 2026-10-28 16:56:13 +0000 pf: Free the stack key when attaching it collides pf_state_key_attach() attaches the wire key first and then the stack key, setting sks to NULL before the second pass. When the stack key collides with an existing state, the error path removes the wire key again but then frees sks, which is NULL by then: the stack key leaks. pfsync hits this. A peer that gets a plain state in a bulk update before a redirected state whose stack key is the plain state's key fails to import the redirected state, leaks its stack key, and leaks another one with every later update of that state. Free sk, the key of the pass that collided (skw on the wire pass), and add a test. Reviewed by: kp Fixes: 371bd29d4b22 ("pf: fix potential state key leak") MFC after: 1 week Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D60951 --- sys/netpfil/pf/pf.c | 11 ++-- tests/sys/netpfil/pf/pfsync.sh | 141 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 6 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index bbba67529d09..f688bf82ddbc 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1872,12 +1872,11 @@ keyattach: PF_SK_WIRE); PF_HASHROW_UNLOCK(ih); KEYS_UNLOCK(); - if (idx == PF_SK_WIRE) - /* - * We've not inserted either key. - * Free both. - */ - uma_zfree(V_pf_state_key_z, skw); + /* + * sk, this pass's key, is not in the + * hash; sks is NULL on the stack pass. + */ + uma_zfree(V_pf_state_key_z, sk); if (skw != sks) uma_zfree( V_pf_state_key_z, diff --git a/tests/sys/netpfil/pf/pfsync.sh b/tests/sys/netpfil/pf/pfsync.sh index c3914383a627..0e1d83d21823 100644 --- a/tests/sys/netpfil/pf/pfsync.sh +++ b/tests/sys/netpfil/pf/pfsync.sh @@ -1742,6 +1742,146 @@ flush_by_intf_cleanup() pfsynct_cleanup } +# The state key zones of all vnets, by sysctl name. +key_zones() +{ + sysctl -N vm.uma | \ + sed -n 's/^vm\.uma\.\(pf_state_keys[_0-9]*\)\.size$/\1/p' +} + +# Count of the states in jail $1 from source ports 40000-40031 whose output +# line matches $2. +port_states() +{ + jexec $1 pfctl -ss | grep '198.51.100.254:400[0-3][0-9]' | \ + grep -c -- "$2" +} + +# Wait up to 10 seconds for "eval $1" to print a number of at least $2. +wait_for() +{ + for i in $(seq 1 20); do + [ "$(eval "$1")" -ge "$2" ] && return 0 + sleep 0.5 + done + atf_fail "Timed out waiting for '$1' to reach $2 (is $(eval "$1"))" +} + +# Send one UDP packet to 198.51.100.1:$1 from each of the source ports +# 40000-40031. +send_flows() +{ + python3 -c ' +import socket, sys +for sport in range(40000, 40032): + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + s.bind(("198.51.100.254", sport)) + s.sendto(b"x", ("198.51.100.1", int(sys.argv[1]))) + s.close() +' $1 +} + +# Number of state records jail $1 has received over pfsync. +state_records() +{ + jexec $1 netstat -s -p pfsync | awk '$2 == "state" && + $3 ~ /^(insert|update)s?$/ && $4 == "received" { n += $1 } + END { print n + 0 }' +} + +atf_test_case "stack_key_collision" "cleanup" +stack_key_collision_head() +{ + atf_set descr 'Failing to import a state must not leak its keys' + atf_set require.user root + atf_set require.progs python3 + # key_zones() would see the zones of vnets other tests create. + atf_set is.exclusive true +} + +stack_key_collision_body() +{ + pfsynct_init + + epair_sync=$(vnet_mkepair) + epair_one=$(vnet_mkepair) + epair_two=$(vnet_mkepair) + + vnet_mkjail one ${epair_one}a ${epair_sync}a + # Each vnet gets its own state key zone: find two's. + zones=$(key_zones) + vnet_mkjail two ${epair_two}a ${epair_sync}b + zone=$(key_zones | grep -vx "${zones}") + if [ -z "${zone}" ] || [ $(echo "${zone}" | wc -l) -ne 1 ]; then + atf_fail "No unique state key zone for jail two: '${zone}'" + fi + + jexec one ifconfig ${epair_sync}a 192.0.2.1/24 up + jexec one ifconfig ${epair_one}a 198.51.100.1/24 up + jexec one ifconfig pfsync0 \ + syncdev ${epair_sync}a \ + up + jexec two ifconfig ${epair_two}a 198.51.100.2/24 up + jexec two ifconfig ${epair_sync}b 192.0.2.2/24 up + + rdr="rdr on ${epair_one}a inet proto udp to port 9999" + jexec one pfctl -e + pft_set_rules one \ + "set skip on ${epair_sync}a" \ + "${rdr} -> 198.51.100.1 port 7" \ + "pass keep state" + jexec two pfctl -e + pft_set_rules two \ + "set skip on { ${epair_sync}b ${epair_two}a }" \ + "pass keep state" + + ifconfig ${epair_one}b 198.51.100.254/24 up + # Resolve one's address before sending more than ARP holds. + atf_check -s exit:0 -o ignore ping -c 1 198.51.100.1 + + # Plain states to port 7, which two gets in a bulk update. + send_flows 7 + wait_for "port_states one ." 32 + jexec two ifconfig pfsync0 \ + syncdev ${epair_sync}b \ + up + wait_for "port_states two ." 32 + + # two stops listening and keeps them; one forgets them. + jexec two ifconfig pfsync0 -syncdev down + jexec one pfctl -F states + + # Redirected states whose stack keys are the plain states' keys. + send_flows 9999 + wait_for "port_states one 9999" 32 + + # two gets them in a second bulk update and cannot import any of + # them: each one's stack key collides with a plain state. Wait for + # the 32 state records themselves: the bulk update's end mark can + # arrive before some of them. + before=$(state_records two) + jexec two ifconfig pfsync0 \ + syncdev ${epair_sync}b \ + up + wait_for "echo \$((\$(state_records two) - ${before}))" 32 + if [ "$(port_states two 9999)" -ne 0 ]; then + atf_fail "two imported a colliding state" + fi + + # With every state gone, so must be every state key. + jexec two ifconfig pfsync0 -syncdev down + jexec two pfctl -F states + keys=$(sysctl -n vm.uma.${zone}.stats.current) + if [ "${keys}" -ne 0 ]; then + atf_fail "${keys} state keys left on two without states" + fi +} + +stack_key_collision_cleanup() +{ + pfsynct_cleanup +} + atf_init_test_cases() { atf_add_test_case "basic" @@ -1767,4 +1907,5 @@ atf_init_test_cases() atf_add_test_case "altq_queues" atf_add_test_case "rt_af" atf_add_test_case "flush_by_intf" + atf_add_test_case "stack_key_collision" }