Thread (1 message) 1 message, 1 author, 2d ago

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

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-09-02 14:18:47
Also in: lkml

On Tue, Sep 1, 2026 at 9:55 PM Qingyu Zhang [off-list ref] 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@intel.com/ (local)

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;
}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help