Re: [BUG] net/sched: cls_u32: ht_down refcount leak on replace_hw failure

[Jamal Hadi Salim]

On Tue, Sep 1, 2026 at 9:55 PM Qingyu Zhang <usupergate@xxxxxxxxx> wrote:
>
> Hello,
>
> u32_change()'s replace-failure path leaks a reference on the linked
> hash table (ht_down). A patch is attached.
>
> Type: memory leak (refcount)
>
> * Summary
>
> u32_init_knode() does refcount_inc(&ht->refcnt) for the copied
> ht_down. On u32_replace_hw_knode() failure:
>
>        u32_unbind_filter(tp, new, tb);
>        if (tb[TCA_U32_LINK]) {
>                ht_old = rtnl_dereference(n->ht_down); /* live knode */
>                if (ht_old)
>                        refcount_inc(&ht_old->refcnt);
>        }
>        __u32_destroy_key(new);  /* already drops new->ht_down */
>
> The extra inc is on the original knode's table. Destroying new
> already accounts for the copy. Net +1 that is never dropped.
> e8d3d78c19be added that inc; it is wrong given ec5b0f605b105.
>
> * Affected
>
> e8d3d78c19be. Reproduced on 08dbfad3f504. Needs cls_u32, a way to
> make u32_replace_hw_knode() fail (no offload device: kretprobe, or
> a driver that returns -EINVAL), and kmemleak.
>
> * Reproduction
>
>        tc qdisc add dev lo ingress
>        tc filter add dev lo parent ffff: handle 800: protocol all u32 divisor 1
>        tc filter add dev lo parent ffff: handle 801: protocol all u32 divisor 1
>        tc filter add ... u32 ht 800: link 801: ...
>        # replace that knode so replace_hw_knode fails
>        tc filter del ...
>        echo scan > /sys/kernel/debug/kmemleak
>
> kmemleak: size 192, u32_change, handle 0x80100000.
>
> Because skip_sw on replace is rejected earlier ("flags do not
> match"), the QEMU PoC uses a kretprobe to force the 2nd
> u32_replace_hw_knode to -EINVAL (poc/fail_replace.c + poc/u32_poc.c).
>

Isnt this the same cutnpaste from here?
https://lore.kernel.org/netdev/20260813122242.1690024-4-jedrzej.jagielski@xxxxxxxxx/

Have your AI check next time instead of overwhelming us on the list.
It is better not to post anything that makes someone spend valuable
cycles going to double-check this slop.

cheers,
jamal

> * Expected
>
> Failed replace does not change ht_down's refcount of the live knode.
>
> * Actual
>
> The 801: hash table stays referenced after qdisc destroy.
>
> Please consider the suggested patch
>
> Thanks.
>
> Suggested patch:
> ```
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index ac6d0fa5a40e..a619dbb435b8 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -938,14 +938,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
> err = u32_replace_hw_knode(tp, new, flags, extack);
> if (err) {
> u32_unbind_filter(tp, new, tb);
> -
> - if (tb[TCA_U32_LINK]) {
> - struct tc_u_hnode *ht_old;
> -
> - ht_old = rtnl_dereference(n->ht_down);
> - if (ht_old)
> - refcount_inc(&ht_old->refcnt);
> - }
> __u32_destroy_key(new);
> return err;
> }
> ```