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

On Fri, Sep 18, 2026 at 1:37 AM Alexandre Ferrieux
<[email protected]> wrote:
>
> Le jeu. 17 sept. 2026 à 14:08, Simon Horman <[email protected]> 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) <[email protected]>
> > > Closes: https://sashiko.dev/#/patchset/[email protected]
> > > Reviewed-by: Victor Nogueira <[email protected]>
> > > Tested-by: hybris <[email protected]>
> > > Signed-off-by: Jamal Hadi Salim <[email protected]>
> >
> > Reviewed-by: Simon Horman <[email protected]>
> >
>
> 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