[PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[Jamal Hadi Salim]

A u32 hash table created with an explicit handle ('tc filter add ...
handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
the destroy paths free it under handle2id(handle). The two key domains
disagree for handles in the 0x800..0xFFF htid range:
handle2id() folds them back into the auto-allocated id space (1..0x7FF).

A manual table therefore leaves its raw-keyed IDR entry unreachable on
delete (a permanent leak), and its delete can drop the idr entry of an
unrelated live auto table. A later auto allocation can then hand out a
handle that aliases the live manual table; u32_lookup_ht() first-match
routes lookups and TCA_U32_LINK for that htid to the wrong table.

Key the divisor-path alloc on handle2id(handle) so allocation and
removal share one key domain. A manual handle that maps onto an id
already in use is rejected with -ENOSPC, and auto allocation skips ids
held by live manual tables.

Conditions to recreate:
  ip link add test0 type dummy
  tc qdisc add dev test0 clsact
  tc filter add dev test0 ingress protocol ip pref 1 \
          handle 801: u32 divisor 16
  tc filter add dev test0 ingress protocol ip pref 2 u32 divisor 16
  tc -d filter show dev test0 ingress | grep 'fh 801:'
  # unpatched: two live tables with handle 0x80100000 (the pref 2 root
  # hnode is auto-allocated id 1); patched: the auto hnode takes id 2.

Also tested with a poc with a live u32 table on the block, add/delete a manual
table 'handle 901: u32 divisor 1' twice; unpatched, the re-add fails with
-ENOSPC because the raw key leaked on the first delete.

Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes.")
Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/cls_u32.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index a3e65c8cf29e..76ce2d124079 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1003,8 +1003,16 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 				return -ENOMEM;
 			}
 		} else {
-			err = idr_alloc_u32(&tp_c->handle_idr, ht, &handle,
-					    handle, GFP_KERNEL);
+			/* The IDR is keyed on the mapped id, and that is
+			 * what the destroy paths remove. Ask for it here,
+			 * so a manual handle colliding with the
+			 * auto-allocated id space is rejected (-ENOSPC)
+			 * instead of aliasing a future auto id.
+			 */
+			u32 id = handle2id(handle);
+
+			err = idr_alloc_u32(&tp_c->handle_idr, ht, &id, id,
+					    GFP_KERNEL);
 			if (err) {
 				kfree(ht);
 				return err;
--
2.43.0

[PATCH net 2/2] selftests/tc-testing: add u32 manual table handle IDR tests

[Jamal Hadi Salim]

35fc: create a manual table with handle 801:, then add an auto-allocated
table. Before the fix, the auto allocation reuses id 1 and hands out the
same handle 0x80100000, aliasing the manual table; the test requires the
manual 801: handle to keep exactly one entry in the dump.

a6e8: with a live u32 table keeping the tc_u_common alive, add and delete
a manual table with handle 901:, then re-add it. Unpatched, the delete
leaks the raw-keyed IDR entry and the re-add fails with -ENOSPC; the
test requires the re-add to succeed.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 .../tc-testing/tc-tests/filters/u32.json      | 48 +++++++++++++++++++
 1 file changed, 48 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 e2b03f2b5e89..edc5148a8d97 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
@@ -376,5 +376,53 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY clsact"
         ]
+    },
+    {
+        "id": "35fc",
+        "name": "u32 manual table then auto table: auto allocation must not alias a live manual handle",
+        "category": [
+            "filter",
+            "u32"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 ingress protocol ip pref 1 handle 801: u32 divisor 16"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip pref 2 u32 divisor 16",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -d filter show dev $DEV1 ingress",
+        "matchPattern": "fh 801:",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "a6e8",
+        "name": "u32 manual table add/del does not leak its idr entry (re-adding the same handle succeeds)",
+        "category": [
+            "filter",
+            "u32"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 ingress protocol ip pref 1 u32 divisor 16",
+            "$TC filter add dev $DEV1 ingress protocol ip pref 5 handle 901: u32 divisor 1",
+            "$TC filter del dev $DEV1 ingress protocol ip pref 5 handle 901: u32"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip pref 6 handle 901: u32 divisor 1",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -d filter show dev $DEV1 ingress",
+        "matchPattern": "fh 901: ht divisor 1",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
     }
 ]
--
2.43.0

Re: [PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[Simon Horman]

On Wed, Sep 16, 2026 at 06:01:14AM -0400, Jamal Hadi Salim wrote:
> A u32 hash table created with an explicit handle ('tc filter add ...
> handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
> the destroy paths free it under handle2id(handle). The two key domains
> disagree for handles in the 0x800..0xFFF htid range:
> handle2id() folds them back into the auto-allocated id space (1..0x7FF).
> 
> A manual table therefore leaves its raw-keyed IDR entry unreachable on
> delete (a permanent leak), and its delete can drop the idr entry of an
> unrelated live auto table. A later auto allocation can then hand out a
> handle that aliases the live manual table; u32_lookup_ht() first-match
> routes lookups and TCA_U32_LINK for that htid to the wrong table.
> 
> Key the divisor-path alloc on handle2id(handle) so allocation and
> removal share one key domain. A manual handle that maps onto an id
> already in use is rejected with -ENOSPC, and auto allocation skips ids
> held by live manual tables.
> 
> Conditions to recreate:
>   ip link add test0 type dummy
>   tc qdisc add dev test0 clsact
>   tc filter add dev test0 ingress protocol ip pref 1 \
>           handle 801: u32 divisor 16
>   tc filter add dev test0 ingress protocol ip pref 2 u32 divisor 16
>   tc -d filter show dev test0 ingress | grep 'fh 801:'
>   # unpatched: two live tables with handle 0x80100000 (the pref 2 root
>   # hnode is auto-allocated id 1); patched: the auto hnode takes id 2.
> 
> Also tested with a poc with a live u32 table on the block, add/delete a manual
> table 'handle 901: u32 divisor 1' twice; unpatched, the re-add fails with
> -ENOSPC because the raw key leaked on the first delete.
> 
> Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes.")
> Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Tested-by: hybris <hybris@mojatatu.ai>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 2/2] selftests/tc-testing: add u32 manual table handle IDR tests

[Simon Horman]

On Wed, Sep 16, 2026 at 06:01:15AM -0400, Jamal Hadi Salim wrote:
> 35fc: create a manual table with handle 801:, then add an auto-allocated
> table. Before the fix, the auto allocation reuses id 1 and hands out the
> same handle 0x80100000, aliasing the manual table; the test requires the
> manual 801: handle to keep exactly one entry in the dump.
> 
> a6e8: with a live u32 table keeping the tc_u_common alive, add and delete
> a manual table with handle 901:, then re-add it. Unpatched, the delete
> leaks the raw-keyed IDR entry and the re-add fails with -ENOSPC; the
> test requires the re-add to succeed.
> 
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Tested-by: hybris <hybris@mojatatu.ai>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 16 Sep 2026 06:01:14 -0400 you wrote:
> A u32 hash table created with an explicit handle ('tc filter add ...
> handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
> the destroy paths free it under handle2id(handle). The two key domains
> disagree for handles in the 0x800..0xFFF htid range:
> handle2id() folds them back into the auto-allocated id space (1..0x7FF).
> 
> A manual table therefore leaves its raw-keyed IDR entry unreachable on
> delete (a permanent leak), and its delete can drop the idr entry of an
> unrelated live auto table. A later auto allocation can then hand out a
> handle that aliases the live manual table; u32_lookup_ht() first-match
> routes lookups and TCA_U32_LINK for that htid to the wrong table.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing
    https://git.kernel.org/netdev/net/c/0a5f5d9e94de
  - [net,2/2] selftests/tc-testing: add u32 manual table handle IDR tests
    https://git.kernel.org/netdev/net/c/960ab631f3d8

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

Re: [PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[Alexandre Ferrieux]

Le jeu. 17 sept. 2026 à 14:08, Simon Horman <horms@kernel.org> a écrit :
>
> On Wed, Sep 16, 2026 at 06:01:14AM -0400, Jamal Hadi Salim wrote:
> > A u32 hash table created with an explicit handle ('tc filter add ...
> > handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
> > the destroy paths free it under handle2id(handle). The two key domains
> > disagree for handles in the 0x800..0xFFF htid range:
> > handle2id() folds them back into the auto-allocated id space (1..0x7FF).
> >
> > A manual table therefore leaves its raw-keyed IDR entry unreachable on
> > delete (a permanent leak), and its delete can drop the idr entry of an
> > unrelated live auto table. A later auto allocation can then hand out a
> > handle that aliases the live manual table; u32_lookup_ht() first-match
> > routes lookups and TCA_U32_LINK for that htid to the wrong table.
> >
> > Key the divisor-path alloc on handle2id(handle) so allocation and
> > removal share one key domain. A manual handle that maps onto an id
> > already in use is rejected with -ENOSPC, and auto allocation skips ids
> > held by live manual tables.
> >
> > Conditions to recreate:
> >   ip link add test0 type dummy
> >   tc qdisc add dev test0 clsact
> >   tc filter add dev test0 ingress protocol ip pref 1 \
> >           handle 801: u32 divisor 16
> >   tc filter add dev test0 ingress protocol ip pref 2 u32 divisor 16
> >   tc -d filter show dev test0 ingress | grep 'fh 801:'
> >   # unpatched: two live tables with handle 0x80100000 (the pref 2 root
> >   # hnode is auto-allocated id 1); patched: the auto hnode takes id 2.
> >
> > Also tested with a poc with a live u32 table on the block, add/delete a manual
> > table 'handle 901: u32 divisor 1' twice; unpatched, the re-add fails with
> > -ENOSPC because the raw key leaked on the first delete.
> >
> > Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes.")
> > Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
> > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > Tested-by: hybris <hybris@mojatatu.ai>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>

Hi Jamal, Simon,

Thanks a lot for the fix.
One nit I'd like to pick is the "Fixes:" header.
Specifically, it mentions my patch:

     Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic
failure to free IDR entries for hnodes.")

However, what this does fix is *not* a regression introduced by the
above patch, but an age-old inconsistency between handle encodings.
So, It would be more accurate to mention an earlier commit:

    Fixes: e7614370d6f0 ("net_sched: use idr to allocate u32 filter handles")

What do you think ?

-Alex

Re: [PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[Jamal Hadi Salim]

On Fri, Sep 18, 2026 at 1:37 AM Alexandre Ferrieux
<alexandre.ferrieux@gmail.com> wrote:
>
> Le jeu. 17 sept. 2026 à 14:08, Simon Horman <horms@kernel.org> a écrit :
> >
> > On Wed, Sep 16, 2026 at 06:01:14AM -0400, Jamal Hadi Salim wrote:
> > > A u32 hash table created with an explicit handle ('tc filter add ...
> > > handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
> > > the destroy paths free it under handle2id(handle). The two key domains
> > > disagree for handles in the 0x800..0xFFF htid range:
> > > handle2id() folds them back into the auto-allocated id space (1..0x7FF).
> > >
> > > A manual table therefore leaves its raw-keyed IDR entry unreachable on
> > > delete (a permanent leak), and its delete can drop the idr entry of an
> > > unrelated live auto table. A later auto allocation can then hand out a
> > > handle that aliases the live manual table; u32_lookup_ht() first-match
> > > routes lookups and TCA_U32_LINK for that htid to the wrong table.
> > >
> > > Key the divisor-path alloc on handle2id(handle) so allocation and
> > > removal share one key domain. A manual handle that maps onto an id
> > > already in use is rejected with -ENOSPC, and auto allocation skips ids
> > > held by live manual tables.
> > >
> > > Conditions to recreate:
> > >   ip link add test0 type dummy
> > >   tc qdisc add dev test0 clsact
> > >   tc filter add dev test0 ingress protocol ip pref 1 \
> > >           handle 801: u32 divisor 16
> > >   tc filter add dev test0 ingress protocol ip pref 2 u32 divisor 16
> > >   tc -d filter show dev test0 ingress | grep 'fh 801:'
> > >   # unpatched: two live tables with handle 0x80100000 (the pref 2 root
> > >   # hnode is auto-allocated id 1); patched: the auto hnode takes id 2.
> > >
> > > Also tested with a poc with a live u32 table on the block, add/delete a manual
> > > table 'handle 901: u32 divisor 1' twice; unpatched, the re-add fails with
> > > -ENOSPC because the raw key leaked on the first delete.
> > >
> > > Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes.")
> > > Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
> > > Closes: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
> > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > Tested-by: hybris <hybris@mojatatu.ai>
> > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >
> > Reviewed-by: Simon Horman <horms@kernel.org>
> >
>
> Hi Jamal, Simon,
>
> Thanks a lot for the fix.
> One nit I'd like to pick is the "Fixes:" header.
> Specifically, it mentions my patch:
>
>      Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic
> failure to free IDR entries for hnodes.")
>
> However, what this does fix is *not* a regression introduced by the
> above patch,

This patch fixes the alloc/remove key-domain asymmetry introduced by
73af53d82076
Prior to your patch, the alloc/remove key domain was symmetric; you
forgot to convert the "else {" part in your conversion, which this
patch fixes. The comment tries to explain this detail.

cheers,
jamal

>but an age-old inconsistency between handle encodings.
> So, It would be more accurate to mention an earlier commit:
>
>     Fixes: e7614370d6f0 ("net_sched: use idr to allocate u32 filter handles")
>
> What do you think ?
>
> -Alex

Re: [PATCH net 1/2] net/sched: cls_u32: fix manual hash table handle IDR aliasing

[Alexandre Ferrieux]

Le ven. 18 sept. 2026 à 10:22, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> On Fri, Sep 18, 2026 at 1:37 AM Alexandre Ferrieux
> <alexandre.ferrieux@gmail.com> wrote:
> >
> > Le jeu. 17 sept. 2026 à 14:08, Simon Horman <horms@kernel.org> a écrit :
> > >
> > > On Wed, Sep 16, 2026 at 06:01:14AM -0400, Jamal Hadi Salim wrote:
> > > > A u32 hash table created with an explicit handle ('tc filter add ...
> > > > handle 801: u32 divisor N') keys its IDR entry on the raw handle, while
> > > > the destroy paths free it under handle2id(handle). The two key domains
> > > > disagree for handles in the 0x800..0xFFF htid range:
> > > > handle2id() folds them back into the auto-allocated id space (1..0x7FF).
> > > >
> > > > A manual table therefore leaves its raw-keyed IDR entry unreachable on
> > > > delete (a permanent leak), and its delete can drop the idr entry of an
> > > > unrelated live auto table. A later auto allocation can then hand out a
> > > > handle that aliases the live manual table; u32_lookup_ht() first-match
> > > > routes lookups and TCA_U32_LINK for that htid to the wrong table.
> > > >
> > > > Key the divisor-path alloc on handle2id(handle) so allocation and
> > > > removal share one key domain. A manual handle that maps onto an id
> > > > already in use is rejected with -ENOSPC, and auto allocation skips ids
> > > > held by live manual tables.
> > > >
> > > > Conditions to recreate:
> > > >   ip link add test0 type dummy
> > > >   tc qdisc add dev test0 clsact
> > > >   tc filter add dev test0 ingress protocol ip pref 1 \
> > > >           handle 801: u32 divisor 16
> > > >   tc filter add dev test0 ingress protocol ip pref 2 u32 divisor 16
> > > >   tc -d filter show dev test0 ingress | grep 'fh 801:'
> > > >   # unpatched: two live tables with handle 0x80100000 (the pref 2 root
> > > >   # hnode is auto-allocated id 1); patched: the auto hnode takes id 2.
> > > >
> > > > Also tested with a poc with a live u32 table on the block, add/delete a manual
> > > > table 'handle 901: u32 divisor 1' twice; unpatched, the re-add fails with
> > > > -ENOSPC because the raw key leaked on the first delete.
> > > >
> > > > Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes.")
> > > > Reported-by: Sashiko (gemini + nipa) <sashiko-bot@kernel.org>
> > > > Closes: https://sashiko.dev/#/patchset/20260822222049.114526-1-jhs@mojatatu.com
> > > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > > Tested-by: hybris <hybris@mojatatu.ai>
> > > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > >
> > > Reviewed-by: Simon Horman <horms@kernel.org>
> > >
> >
> > Hi Jamal, Simon,
> >
> > Thanks a lot for the fix.
> > One nit I'd like to pick is the "Fixes:" header.
> > Specifically, it mentions my patch:
> >
> >      Fixes: 73af53d82076 ("net: sched: cls_u32: Fix u32's systematic
> > failure to free IDR entries for hnodes.")
> >
> > However, what this does fix is *not* a regression introduced by the
> > above patch,
>
> This patch fixes the alloc/remove key-domain asymmetry introduced by
> 73af53d82076
> Prior to your patch, the alloc/remove key domain was symmetric; you
> forgot to convert the "else {" part in your conversion, which this
> patch fixes. The comment tries to explain this detail.
>
> cheers,
> jamal

Oh, I stand corrected. Thank you !

-Alex