[PATCH net v3 1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted

[PATCH net v3 1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted

[Jamal Hadi Salim]

gen_new_kid() falls back to returning max (htid | 0xFFF) when both
idr_alloc_u32() ranges are full, instead of reporting an error.
u32_change() trusts that value and inserts a new knode with a handle
that is already live in the hash table, breaking handle uniqueness
within the table's node ID space.

The handle was never reserved in ht->handle_idr, so every later error
path that does idr_remove(&ht->handle_idr, handle) removes the
reservation of a different, live knode, which is then reused — one
failed add compounds into further duplicates.

The 4095 limit is per (table, bucket) — ht->handle_idr is per hash
table and the range is derived from htid (bucketid), so a table with
divisor 256 can legitimately hold 256*4095 knodes.

The sibling helper gen_new_htid() has the same silent in-band failure:
it returns 0 when the tp_c handle pool (1..0x7FF) is full, and
u32_init() publishes the root hash table with handle 0 without
checking.  Two root tables with handle 0 alias in u32_lookup_ht(),
allowing cross-tcf_proto knode add/lookup/delete.  Add the same
exhaustion check that the divisor path already has.

Return an error so u32_change() fails with ENOSPC/ENOMEM when the
node ID space is exhausted, and so u32_init() fails with -ENOMEM
when the hash table ID space is exhausted.  The extack message
distinguishes pool exhaustion (-ENOSPC) from a transient allocation
failure (-ENOMEM).

Conditions to recreate the bug:
- CONFIG_NET_SCHED=y, CONFIG_CLS_U32=y (or =m with module loaded)
- Create a clsact qdisc on a device, then add 4095 u32 filters with
  auto-generated handles to fill the node ID space for the root hash
  table (single bucket). The 4096th auto-handle filter add triggers
  the duplicate handle (fh 800::fff reused). Reachable at Level 2
  (unshare -Urn, namespace-local CAP_NET_ADMIN).
- For gen_new_htid: create 2047 u32 proto entries on the same block
  to fill the tp_c handle pool, then create one more. The root table
  gets handle 0 and aliases with other handle-0 root tables.

Fixes: 7801db8aec95 ("net_sched: avoid generating same handle for u32 filters")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v2 -> v3:
- Fixed tdc test that sashiko (correctly) pointed potential security
  issue on.
- extack: condition the "Hash table node ID pool exhausted" message on
  -ENOSPC; emit a neutral "Failed to allocate node ID" for -ENOMEM
  Introduce small extack helper. The v2 message was misleading for
  -ENOMEM (Sashiko nipa gpt-5-6-sol-1-2).

Sashiko links (v2 RESEND reviews that prompted v3):
- gemini: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
- nipa: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822222049.114526-1-jhs@mojatatu.com

v1 -> v2:
- Commit message: removed misleading "4095-knode cap" claim (knodes is a
  liveness counter, not a limit; 4095 is per (table, bucket) via
  ht->handle_idr/htid); reworded to "handle uniqueness within node ID
  space"(sashiko).
- Commit message: added IDR-desync paragraph (max handle never reserved
  -> erridr removes other live knode's reservation -> reuse compounds).
- Commit message: noted ENOSPC/ENOMEM both reachable (idr_get_free/
  radix_tree_extend).
- Fixes: e7614370d6f04 -> 7801db8aec95 (duplicate-handle bug predates
  the IDR conversion; e761 only added the IDR-desync consequence).
- Added NL_SET_ERR_MSG_MOD(extack, "Hash table node ID pool exhausted")
  at both gen_new_kid() call sites; bare -ENOSPC -> "No space left on
  device" was confusing.
- Folded gen_new_htid() exhaustion check in u32_init() (Sashiko nipa
  main-1-0): same silent in-band failure pattern — gen_new_htid()
  returns 0 when tp_c handle pool full, u32_init() published root table
  with handle 0 without checking, causing cross-tcf_proto aliasing.
- Renewed tdc test (v1 tdc failed in long-running sweep, prompting v2).

Sashiko links (v1 reviews that prompted v2):
- gemini: https://sashiko.dev/#/patchset/20260820095236.68390-1-jhs@mojatatu.com
- nipa: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260820095236.68390-1-jhs@mojatatu.com
---
 net/sched/cls_u32.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index ac6d0fa5a40e..a3e65c8cf29e 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -370,6 +370,10 @@ static int u32_init(struct tcf_proto *tp)
 
 	refcount_set(&root_ht->refcnt, 1);
 	root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : id2handle(0);
+	if (root_ht->handle == 0) {
+		kfree(root_ht);
+		return -ENOMEM;
+	}
 	root_ht->prio = tp->prio;
 	root_ht->is_root = true;
 	idr_init(&root_ht->handle_idr);
@@ -695,21 +699,33 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
 	return ret;
 }
 
-static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid)
+static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid, int *err)
 {
 	u32 index = htid | 0x800;
 	u32 max = htid | 0xFFF;
 
+	*err = 0;
+
 	if (idr_alloc_u32(&ht->handle_idr, NULL, &index, max, GFP_KERNEL)) {
 		index = htid + 1;
-		if (idr_alloc_u32(&ht->handle_idr, NULL, &index, max,
-				 GFP_KERNEL))
-			index = max;
+		*err = idr_alloc_u32(&ht->handle_idr, NULL, &index, max,
+				     GFP_KERNEL);
+		if (*err)
+			return 0;
 	}
 
 	return index;
 }
 
+static int u32_kid_extack(int err, struct netlink_ext_ack *extack)
+{
+	if (err == -ENOSPC)
+		NL_SET_ERR_MSG_MOD(extack, "Hash table node ID pool exhausted");
+	else
+		NL_SET_ERR_MSG_MOD(extack, "Failed to allocate node ID");
+	return err;
+}
+
 static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
 	[TCA_U32_CLASSID]	= { .type = NLA_U32 },
 	[TCA_U32_HASH]		= { .type = NLA_U32 },
@@ -1079,7 +1095,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		 * handle which is used to uniquely identify the match entry.
 		 */
 		if (!TC_U32_NODE(handle)) {
-			handle = gen_new_kid(ht, htid);
+			handle = gen_new_kid(ht, htid, &err);
+			if (err)
+				return u32_kid_extack(err, extack);
 		} else {
 			handle = htid | TC_U32_NODE(handle);
 			err = idr_alloc_u32(&ht->handle_idr, NULL, &handle,
@@ -1091,7 +1109,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		/* The user did not give us a handle; lets just generate one
 		 * from the table's pool of nodeids.
 		 */
-		handle = gen_new_kid(ht, htid);
+		handle = gen_new_kid(ht, htid, &err);
+		if (err)
+			return u32_kid_extack(err, extack);
 	}
 
 	if (tb[TCA_U32_SEL] == NULL) {
-- 
2.43.0

[PATCH net v3 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test

[Jamal Hadi Salim]

Add a tdc test case that fills the u32 node ID space with 4095
auto-generated handles, then attempts to add a 4096th. On the fixed
kernel the 4096th filter is rejected with ENOSPC (exit 2). On the
unfixed kernel it silently succeeds with a duplicate handle.

The setup pipes the 4095 add commands directly into `tc -b -` inside a
single bash -c (matching the existing test id 1234 pattern), avoiding
any temp file.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v2 -> v3:
- Dropped the /tmp/u32_batch.txt temp file. The setup now pipes the
  4095 add commands directly into `$TC -b -` inside a single bash -c
  (matching the existing test id 1234 pattern), eliminating the
  symlink/TOCTOU risk and the leftover-file cleanup gap flagged by
  both Sashikos (gemini Medium + nipa gpt-5-6-sol-6-9, gpt-5-6-sol-4-7).
  New auto-generated tdc id: 70fd.

v1 -> v2:
- Renewed tdc test (v1 tdc failed in long-running sweep, prompting v2).
---
 .../tc-testing/tc-tests/filters/u32.json      | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json b/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
index b2ca9d4e991b..e2b03f2b5e89 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
@@ -353,5 +353,28 @@
         "teardown": [
             "$TC qdisc del dev $DEV1 parent root drr"
         ]
+    },
+    {
+        "id": "70fd",
+        "name": "Add u32 filter when node ID pool is exhausted (4096th filter rejected)",
+        "category": [
+            "filter",
+            "u32"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY clsact",
+            "bash -c 'for i in {1..4095}; do echo filter add dev $DUMMY ingress prio 1 protocol ip u32 match u8 0 0 at 0; done | $TC -b -'"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DUMMY ingress prio 1 protocol ip u32 match u8 0 0 at 0",
+        "expExitCode": "2",
+        "verifyCmd": "$TC -d filter show dev $DUMMY ingress",
+        "matchPattern": "fh 800::",
+        "matchCount": "4095",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY clsact"
+        ]
     }
 ]
-- 
2.43.0

Re: [PATCH net v3 1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted

[Paolo Abeni]

On 8/25/26 10:10 AM, Jamal Hadi Salim wrote:
> gen_new_kid() falls back to returning max (htid | 0xFFF) when both
> idr_alloc_u32() ranges are full, instead of reporting an error.
> u32_change() trusts that value and inserts a new knode with a handle
> that is already live in the hash table, breaking handle uniqueness
> within the table's node ID space.
> 
> The handle was never reserved in ht->handle_idr, so every later error
> path that does idr_remove(&ht->handle_idr, handle) removes the
> reservation of a different, live knode, which is then reused — one
> failed add compounds into further duplicates.
> 
> The 4095 limit is per (table, bucket) — ht->handle_idr is per hash
> table and the range is derived from htid (bucketid), so a table with
> divisor 256 can legitimately hold 256*4095 knodes.
> 
> The sibling helper gen_new_htid() has the same silent in-band failure:
> it returns 0 when the tp_c handle pool (1..0x7FF) is full, and
> u32_init() publishes the root hash table with handle 0 without
> checking.  Two root tables with handle 0 alias in u32_lookup_ht(),
> allowing cross-tcf_proto knode add/lookup/delete.  Add the same
> exhaustion check that the divisor path already has.
> 
> Return an error so u32_change() fails with ENOSPC/ENOMEM when the
> node ID space is exhausted, and so u32_init() fails with -ENOMEM
> when the hash table ID space is exhausted.  The extack message
> distinguishes pool exhaustion (-ENOSPC) from a transient allocation
> failure (-ENOMEM).
> 
> Conditions to recreate the bug:
> - CONFIG_NET_SCHED=y, CONFIG_CLS_U32=y (or =m with module loaded)
> - Create a clsact qdisc on a device, then add 4095 u32 filters with
>   auto-generated handles to fill the node ID space for the root hash
>   table (single bucket). The 4096th auto-handle filter add triggers
>   the duplicate handle (fh 800::fff reused). Reachable at Level 2
>   (unshare -Urn, namespace-local CAP_NET_ADMIN).
> - For gen_new_htid: create 2047 u32 proto entries on the same block
>   to fill the tp_c handle pool, then create one more. The root table
>   gets handle 0 and aliases with other handle-0 root tables.
> 
> Fixes: 7801db8aec95 ("net_sched: avoid generating same handle for u32 filters")
> Reported-by: vega@nebusec.ai
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> v2 -> v3:
> - Fixed tdc test that sashiko (correctly) pointed potential security
>   issue on.
> - extack: condition the "Hash table node ID pool exhausted" message on
>   -ENOSPC; emit a neutral "Failed to allocate node ID" for -ENOMEM
>   Introduce small extack helper. The v2 message was misleading for
>   -ENOMEM (Sashiko nipa gpt-5-6-sol-1-2).
It looks like that sashiko was able to think more about this patch and
found new stuff:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081052.133898-1-jhs%40mojatatu.com

I'm unsure if that falls under the 'same bug' category and should
addressed here or separately. WDYT?

/P

Re: [PATCH net v3 1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted

[Jamal Hadi Salim]

On Thu, Aug 27, 2026 at 6:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/25/26 10:10 AM, Jamal Hadi Salim wrote:
> > gen_new_kid() falls back to returning max (htid | 0xFFF) when both
> > idr_alloc_u32() ranges are full, instead of reporting an error.
> > u32_change() trusts that value and inserts a new knode with a handle
> > that is already live in the hash table, breaking handle uniqueness
> > within the table's node ID space.
> >
> > The handle was never reserved in ht->handle_idr, so every later error
> > path that does idr_remove(&ht->handle_idr, handle) removes the
> > reservation of a different, live knode, which is then reused — one
> > failed add compounds into further duplicates.
> >
> > The 4095 limit is per (table, bucket) — ht->handle_idr is per hash
> > table and the range is derived from htid (bucketid), so a table with
> > divisor 256 can legitimately hold 256*4095 knodes.
> >
> > The sibling helper gen_new_htid() has the same silent in-band failure:
> > it returns 0 when the tp_c handle pool (1..0x7FF) is full, and
> > u32_init() publishes the root hash table with handle 0 without
> > checking.  Two root tables with handle 0 alias in u32_lookup_ht(),
> > allowing cross-tcf_proto knode add/lookup/delete.  Add the same
> > exhaustion check that the divisor path already has.
> >
> > Return an error so u32_change() fails with ENOSPC/ENOMEM when the
> > node ID space is exhausted, and so u32_init() fails with -ENOMEM
> > when the hash table ID space is exhausted.  The extack message
> > distinguishes pool exhaustion (-ENOSPC) from a transient allocation
> > failure (-ENOMEM).
> >
> > Conditions to recreate the bug:
> > - CONFIG_NET_SCHED=y, CONFIG_CLS_U32=y (or =m with module loaded)
> > - Create a clsact qdisc on a device, then add 4095 u32 filters with
> >   auto-generated handles to fill the node ID space for the root hash
> >   table (single bucket). The 4096th auto-handle filter add triggers
> >   the duplicate handle (fh 800::fff reused). Reachable at Level 2
> >   (unshare -Urn, namespace-local CAP_NET_ADMIN).
> > - For gen_new_htid: create 2047 u32 proto entries on the same block
> >   to fill the tp_c handle pool, then create one more. The root table
> >   gets handle 0 and aliases with other handle-0 root tables.
> >
> > Fixes: 7801db8aec95 ("net_sched: avoid generating same handle for u32 filters")
> > Reported-by: vega@nebusec.ai
> > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> > v2 -> v3:
> > - Fixed tdc test that sashiko (correctly) pointed potential security
> >   issue on.
> > - extack: condition the "Hash table node ID pool exhausted" message on
> >   -ENOSPC; emit a neutral "Failed to allocate node ID" for -ENOMEM
> >   Introduce small extack helper. The v2 message was misleading for
> >   -ENOMEM (Sashiko nipa gpt-5-6-sol-1-2).
> It looks like that sashiko was able to think more about this patch and
> found new stuff:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825081052.133898-1-jhs%40mojatatu.com
>
> I'm unsure if that falls under the 'same bug' category and should
> addressed here or separately. WDYT?

Separately. The Sashiko findings on v3 are the same pre-existing
issues it flagged on v2.
All three are different root causes (IDR key mismatch, refcount
management) from the silent exhaustion failure this patch fixes.

I am tracking them as separate follow-up patches for net-next. Here's
the list just from this one patch from the sashikos, none of them
wrong, but none "regression" and worth fixing for net in my opinion:

1. handle2id() alloc/remove key asymmetry in the divisor path causes
hnode handle duplication
2. ht_down refcount leak on new knode hw offload failure errunbind cleanup
3. ht_down spurious refcount_inc on update knode hw offload failure

There's a bunch of minor ones, that may be worth fixing for niceness
but as you know no good deed goes unpunished with sashiko, the moment
i post a new patch it will find something to complain about.

These are:
- Commit message range "1..0x7FF"   being inaccurate - which is true.
- Extack should say "bucket" not "table"  - which is a minor refinement
- tdc test to check and assert for ENOSPC, i think the explanation it
made is sensible. We only check for failures not which exact return
code it made, but if i change this it will meaning changing a good
number of tdc tests to follow the same approach.

I could probably sneak some of these in in net-next or i could just
resend only fixing this. What says you?

cheers,
jamal

> /P
>

Re: [PATCH net v3 1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 25 Aug 2026 04:10:51 -0400 you wrote:
> gen_new_kid() falls back to returning max (htid | 0xFFF) when both
> idr_alloc_u32() ranges are full, instead of reporting an error.
> u32_change() trusts that value and inserts a new knode with a handle
> that is already live in the hash table, breaking handle uniqueness
> within the table's node ID space.
> 
> The handle was never reserved in ht->handle_idr, so every later error
> path that does idr_remove(&ht->handle_idr, handle) removes the
> reservation of a different, live knode, which is then reused — one
> failed add compounds into further duplicates.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] net/sched: cls_u32: fix duplicate handle when node ID pool is exhausted
    https://git.kernel.org/netdev/net/c/d7e7e98d23f4
  - [net,v3,2/2] selftests: tc-testing: add u32 node ID pool exhaustion test
    https://git.kernel.org/netdev/net/c/7b120a771943

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html