Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/sched: defer qdisc freeing after failed creation
@ 2026-08-05 10:25 David Lee
  2026-08-05 18:00 ` Jamal Hadi Salim
  0 siblings, 1 reply; 2+ messages in thread
From: David Lee @ 2026-08-05 10:25 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, jhs, jiri
  Cc: Kyle Zeng, Dominik 'Disconnect3d' Czarnota, horms, netdev,
	linux-kernel, David Lee

From: Kyle Zeng <kylebot@openai.com>

A qdisc's init callback can publish state to RCU readers before
qdisc_create() completes. In particular, clsact_init() binds a populated
shared ingress block and installs an embedded mini_Qdisc in
dev->tcx_ingress. If subsequent rate estimator setup fails, the unwind
removes that pointer but qdisc_free() immediately releases the qdisc and
its per-CPU statistics. A reader that obtained the miniq before removal
can then access freed memory.

Add qdisc_free_rcu() and use it for the creation error path, matching
normal qdisc destruction. This keeps the embedded miniq and the per-CPU
statistics alive until pre-existing readers complete.

Fixes: 51ab2994c387 ("net: sched: allow ingress and clsact qdiscs to share filter blocks")
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
---
Bug found and triaged by OpenAI Security Research and
validated by Trail of Bits.

Trail of Bits has a reproducer for this bug that triggers a
KASAN use-after-free and can share if needed.

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

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 45a1e8c782..d45442c926 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -793,6 +793,7 @@ 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 668bcd60d1..041bd60072 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1373,7 +1373,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 ef2b4bf515..86d551fbab 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -1103,6 +1103,11 @@ static void qdisc_free_cb(struct rcu_head *head)
 	qdisc_free(q);
 }
 
+void qdisc_free_rcu(struct Qdisc *qdisc)
+{
+	call_rcu(&qdisc->rcu, qdisc_free_cb);
+}
+
 static void __qdisc_destroy(struct Qdisc *qdisc)
 {
 	const struct Qdisc_ops  *ops = qdisc->ops;
@@ -1127,7 +1132,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.53.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net/sched: defer qdisc freeing after failed creation
  2026-08-05 10:25 [PATCH net] net/sched: defer qdisc freeing after failed creation David Lee
@ 2026-08-05 18:00 ` Jamal Hadi Salim
  0 siblings, 0 replies; 2+ messages in thread
From: Jamal Hadi Salim @ 2026-08-05 18:00 UTC (permalink / raw)
  To: David Lee
  Cc: davem, edumazet, kuba, pabeni, jiri, Kyle Zeng,
	Dominik 'Disconnect3d' Czarnota, horms, netdev,
	linux-kernel

On Wed, Aug 5, 2026 at 6:25 AM David Lee <david.lee@trailofbits.com> wrote:
>
> From: Kyle Zeng <kylebot@openai.com>
>
> A qdisc's init callback can publish state to RCU readers before
> qdisc_create() completes. In particular, clsact_init() binds a populated
> shared ingress block and installs an embedded mini_Qdisc in
> dev->tcx_ingress. If subsequent rate estimator setup fails, the unwind
> removes that pointer but qdisc_free() immediately releases the qdisc and
> its per-CPU statistics. A reader that obtained the miniq before removal
> can then access freed memory.
>
> Add qdisc_free_rcu() and use it for the creation error path, matching
> normal qdisc destruction. This keeps the embedded miniq and the per-CPU
> statistics alive until pre-existing readers complete.
>
> Fixes: 51ab2994c387 ("net: sched: allow ingress and clsact qdiscs to share filter blocks")
> Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> Signed-off-by: David Lee <david.lee@trailofbits.com>

Thanks for finding the issue. But you should know the deal by now,
send the poc - you can send it in private. Same goes for your other
patch.

cheers,
jamal

> ---
> Bug found and triaged by OpenAI Security Research and
> validated by Trail of Bits.
>
> Trail of Bits has a reproducer for this bug that triggers a
> KASAN use-after-free and can share if needed.
>
>  include/net/sch_generic.h | 1 +
>  net/sched/sch_api.c       | 2 +-
>  net/sched/sch_generic.c   | 7 ++++++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 45a1e8c782..d45442c926 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -793,6 +793,7 @@ 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 668bcd60d1..041bd60072 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1373,7 +1373,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 ef2b4bf515..86d551fbab 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -1103,6 +1103,11 @@ static void qdisc_free_cb(struct rcu_head *head)
>         qdisc_free(q);
>  }
>
> +void qdisc_free_rcu(struct Qdisc *qdisc)
> +{
> +       call_rcu(&qdisc->rcu, qdisc_free_cb);
> +}
> +
>  static void __qdisc_destroy(struct Qdisc *qdisc)
>  {
>         const struct Qdisc_ops  *ops = qdisc->ops;
> @@ -1127,7 +1132,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.53.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-05 18:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 10:25 [PATCH net] net/sched: defer qdisc freeing after failed creation David Lee
2026-08-05 18:00 ` Jamal Hadi Salim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox