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

On Thu, Aug 27, 2026 at 6:30 AM Paolo Abeni [off-list ref] wrote:

On 8/25/26 10:10 AM, Jamal Hadi Salim wrote:

quoted

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 <redacted>
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