[PATCH net v2 0/1] net/sched: defer qdisc freeing after failed creation

[PATCH net v2 0/1] net/sched: defer qdisc freeing after failed creation

[Weiming Shi]

This is v2 of the qdisc failed-creation lifetime fix posted by Kyle Zeng
and David Lee [1].  The public reproducer is available in a follow-up
message [2].

When clsact binds a populated shared ingress block, ->init() can publish
an embedded mini_Qdisc before qdisc_create() completes.  A later invalid
TCA_RATE makes estimator setup fail.  The unwind removes the published
pointer but frees the containing qdisc synchronously while tc_run() can
still hold it.

The fix itself is unchanged: failed qdiscs are retired through RCU.

With the public reproducer from [2], the vulnerable kernel reports a
KASAN UAF in tc_run(), with the free in qdisc_create().  The same test
completes without a KASAN report on v2.

Changes in v2:
- inline qdisc_free() into qdisc_free_cb()
- use qdisc_free_rcu() for normal and failed-construction teardown
- retain Fixes: 51ab2994c387 as requested
- credit the earlier security-list reporter

[1] https://lore.kernel.org/netdev/20260805102505.740806-1-david.lee@trailofbits.com/
[2] https://lore.kernel.org/netdev/CAC_etQFk%3DtGLvbHPmQwOyrYWgRtDru3dSw4T7sZ-9hvVMy_J-g@mail.gmail.com/

Weiming Shi (1):
  net/sched: defer qdisc freeing after failed creation

 include/net/sch_generic.h |  2 +-
 net/sched/sch_api.c       |  2 +-
 net/sched/sch_generic.c   | 20 ++++++++++----------
 3 files changed, 12 insertions(+), 12 deletions(-)

[PATCH net v2 1/1] net/sched: defer qdisc freeing after failed creation

[Weiming Shi]

An RTM_NEWQDISC request can make clsact bind a populated shared ingress
block during ->init(), publishing an embedded mini_Qdisc to lockless
readers.  If the same request has an invalid TCA_RATE, estimator setup
fails after ->init(); the unwind removes the pointer but synchronously
frees its containing qdisc while tc_run() may still hold it.

Retire failed qdiscs through the same RCU helper as normal destruction.
Inline the synchronous free into the callback now that no direct callers
remain.

Fixes: 51ab2994c387 ("net: sched: allow ingress and clsact qdiscs to share filter blocks")
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/netdev/20260805102505.740806-1-david.lee@trailofbits.com/
Assisted-by: OpenAI-Codex:gpt-5.6
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
Changes in v2:
- inline qdisc_free() into qdisc_free_cb()
- use qdisc_free_rcu() for normal and failed-construction teardown
- use the precise 51ab2994c387 Fixes tag requested on the v1 thread
- credit the earlier security-list reporter and link the public v1

 include/net/sch_generic.h |  2 +-
 net/sched/sch_api.c       |  2 +-
 net/sched/sch_generic.c   | 20 ++++++++++----------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index cbc248776511..f35bd06a6bad 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -793,7 +793,7 @@ void qdisc_offload_query_caps(struct net_device *dev,
 struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 			  const struct Qdisc_ops *ops,
 			  struct netlink_ext_ack *extack);
-void qdisc_free(struct Qdisc *qdisc);
+void qdisc_free_rcu(struct Qdisc *qdisc);
 struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
 				const struct Qdisc_ops *ops, u32 parentid,
 				struct netlink_ext_ack *extack);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 65b35528d125..795accd1bb33 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1382,7 +1382,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 err_out3:
 	qdisc_lock_uninit(sch, ops);
 	netdev_put(dev, &sch->dev_tracker);
-	qdisc_free(sch);
+	qdisc_free_rcu(sch);
 err_out2:
 	bpf_module_put(ops, ops->owner);
 err_out:
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index ef2b4bf51564..71630b795f9d 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -1086,21 +1086,21 @@ void qdisc_reset(struct Qdisc *qdisc)
 }
 EXPORT_SYMBOL(qdisc_reset);
 
-void qdisc_free(struct Qdisc *qdisc)
+static void qdisc_free_cb(struct rcu_head *head)
 {
-	if (qdisc_is_percpu_stats(qdisc)) {
-		free_percpu(qdisc->cpu_bstats);
-		free_percpu(qdisc->cpu_qstats);
+	struct Qdisc *q = container_of(head, struct Qdisc, rcu);
+
+	if (qdisc_is_percpu_stats(q)) {
+		free_percpu(q->cpu_bstats);
+		free_percpu(q->cpu_qstats);
 	}
 
-	kfree(qdisc);
+	kfree(q);
 }
 
-static void qdisc_free_cb(struct rcu_head *head)
+void qdisc_free_rcu(struct Qdisc *qdisc)
 {
-	struct Qdisc *q = container_of(head, struct Qdisc, rcu);
-
-	qdisc_free(q);
+	call_rcu(&qdisc->rcu, qdisc_free_cb);
 }
 
 static void __qdisc_destroy(struct Qdisc *qdisc)
@@ -1127,7 +1127,7 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
 
 	trace_qdisc_destroy(qdisc);
 
-	call_rcu(&qdisc->rcu, qdisc_free_cb);
+	qdisc_free_rcu(qdisc);
 }
 
 void qdisc_destroy(struct Qdisc *qdisc)
-- 
2.55.0