Re: [PATCH net] net/sched: defer qdisc freeing after failed creation
Jamal Hadi Salim <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAM0EoMkMcpE4JZdroYvVuPRzUC8Xwbwv3p4+qkABj=Qk55kWvg@mail.gmail.com> |
On Wed, Aug 5, 2026 at 6:25 AM David Lee <[email protected]> wrote: > > From: Kyle Zeng <[email protected]> > > 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 <[email protected]> > Signed-off-by: David Lee <[email protected]> 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