[PATCH] net/sched: cls_u32: drop ht_down ref on hardware knode failure

[PATCH] net/sched: cls_u32: drop ht_down ref on hardware knode failure

[Noah Weinberger]

Hello,

u32_set_parms() takes a reference on the TCA_U32_LINK hash table
(refcount_inc at the end of the link block), but two of the three
paths that can fail after that point never drop it:

1. Fresh knode (u32_change): when u32_replace_hw_knode() fails, the
   errunbind path unbinds the filter and then frees the node with
   plain kfree(); n->ht_down keeps its reference forever.

2. Replace path: u32_set_parms() has already re-pointed new->ht_down
to the newly linked table B and dropped the old table A, but the
failure block then takes an extra refcount_inc() on A (the old
node's ht_down) as if __u32_destroy_key(new) would drop A - it drops
B instead. The extra reference on A is never released, and since the
old node survives this error path holding its original reference,
A's refcount is permanently one too high.

Each failed attempt (a TC_SETUP_CLSU32-capable driver rejecting the
rule) therefore leaks one reference; the linked table can never be
deleted afterwards (u32_delete returns -EBUSY while refcount > 1) and
is never freed with its hash array.

The patch drops n->ht_down on the fresh-node error path and removes
the bogus compensating increment on the replace path, leaving
__u32_destroy_key(new) as the single place that drops what the node
holds - which is correct for both the LINK and no-LINK cases.

Affected version/commit: mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08), net/sched/cls_u32.c:794 (refcount_inc in u32_set_parms), :958-965 (replace path: compensating refcount_inc targets the old node's table while __u32_destroy_key drops new->ht_down, leaking one reference), :1191-1202 (fresh path: u32_set_parms then u32_replace_hw_knode), :1222-1238 (errunbind path frees n with plain kfree() and never drops n->ht_down), :406-414 (__u32_destroy_key drops n->ht_down)
Longstanding error-path bug (the compensating-inc pattern predates the reviewed window).

This was found during an assisted (AI-supported) static source review of
mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08). The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.

---
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 91f4e64587..82b15ce312 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -955,13 +955,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		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;
 		}
@@ -1221,6 +1214,12 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 
 errunbind:
 	u32_unbind_filter(tp, n, tb);
+	if (tb[TCA_U32_LINK]) {
+		struct tc_u_hnode *ht = rtnl_dereference(n->ht_down);
+
+		if (ht)
+			refcount_dec(&ht->refcnt);
+	}
 
 #ifdef CONFIG_CLS_U32_MARK
 	free_percpu(n->pcpu_success);

Re: [PATCH] net/sched: cls_u32: drop ht_down ref on hardware knode failure

[Paolo Abeni]

On 11/9/26 00:51, Noah Weinberger wrote:
> Hello,
> 
> u32_set_parms() takes a reference on the TCA_U32_LINK hash table
> (refcount_inc at the end of the link block), but two of the three
> paths that can fail after that point never drop it:
> 
> 1. Fresh knode (u32_change): when u32_replace_hw_knode() fails, the
>     errunbind path unbinds the filter and then frees the node with
>     plain kfree(); n->ht_down keeps its reference forever.
> 
> 2. Replace path: u32_set_parms() has already re-pointed new->ht_down
> to the newly linked table B and dropped the old table A, but the
> failure block then takes an extra refcount_inc() on A (the old
> node's ht_down) as if __u32_destroy_key(new) would drop A - it drops
> B instead. The extra reference on A is never released, and since the
> old node survives this error path holding its original reference,
> A's refcount is permanently one too high.
> 
> Each failed attempt (a TC_SETUP_CLSU32-capable driver rejecting the
> rule) therefore leaks one reference; the linked table can never be
> deleted afterwards (u32_delete returns -EBUSY while refcount > 1) and
> is never freed with its hash array.
> 
> The patch drops n->ht_down on the fresh-node error path and removes
> the bogus compensating increment on the replace path, leaving
> __u32_destroy_key(new) as the single place that drops what the node
> holds - which is correct for both the LINK and no-LINK cases.
> 
> Affected version/commit: mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08), net/sched/cls_u32.c:794 (refcount_inc in u32_set_parms), :958-965 (replace path: compensating refcount_inc targets the old node's table while __u32_destroy_key drops new->ht_down, leaking one reference), :1191-1202 (fresh path: u32_set_parms then u32_replace_hw_knode), :1222-1238 (errunbind path frees n with plain kfree() and never drops n->ht_down), :406-414 (__u32_destroy_key drops n->ht_down)
> Longstanding error-path bug (the compensating-inc pattern predates the reviewed window).
> 
> This was found during an assisted (AI-supported) static source review of
> mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08). The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
You should invest a little more time here, asking the LLM model to generate
a reproduced, and actually testing the patch vs it. ATM we are flooded with
tons of reports of mixed quality. Half-hearted theoretical things will
have hard time to get any actual processing. Also you should cc the netdev
ML.

/P