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

[Jamal Hadi Salim]

On Wed, Aug 5, 2026 at 6:25 AM David Lee <david.lee@xxxxxxxxxxxxxxx> wrote:
>
> From: Kyle Zeng <kylebot@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: David Lee <david.lee@xxxxxxxxxxxxxxx>

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