Re: [PATCH net v5 1/2] net/sched: taprio: fix NULL pointer dereference in class dump

Weiming Shi
2026-04-26T13:01:35Z

On 26-04-26 06:59, Jamal Hadi Salim wrote:
> On Wed, Apr 22, 2026 at 12:20 PM Weiming Shi <bestswngs@gmail.com> wrote:
> >
> > When a TAPRIO child qdisc is deleted via RTM_DELQDISC, taprio_graft()
> > is called with new == NULL and stores NULL into q->qdiscs[cl - 1].
> > Subsequent RTM_GETTCLASS dump operations walk all classes via
> > taprio_walk() and call taprio_dump_class(), which calls taprio_leaf()
> > returning the NULL pointer, then dereferences it to read child->handle,
> > causing a kernel NULL pointer dereference.
> >
> > The bug is reachable with namespace-scoped CAP_NET_ADMIN on any kernel
> > with CONFIG_NET_SCH_TAPRIO enabled. On systems with unprivileged user
> > namespaces enabled, an unprivileged local user can trigger a kernel
> > panic by creating a taprio qdisc inside a new network namespace,
> > grafting an explicit child qdisc, deleting it, and requesting a class
> > dump. The RTM_GETTCLASS dump itself requires no capability.
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000007: 0000 [#1] SMP KASAN NOPTI
> > KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
> > RIP: 0010:taprio_dump_class (net/sched/sch_taprio.c:2478)
> > Call Trace:
> > <TASK>
> > tc_fill_tclass (net/sched/sch_api.c:1966)
> > qdisc_class_dump (net/sched/sch_api.c:2326)
> > taprio_walk (net/sched/sch_taprio.c:2514)
> > tc_dump_tclass_qdisc (net/sched/sch_api.c:2352)
> > tc_dump_tclass_root (net/sched/sch_api.c:2370)
> > tc_dump_tclass (net/sched/sch_api.c:2431)
> > rtnl_dumpit (net/core/rtnetlink.c:6864)
> > netlink_dump (net/netlink/af_netlink.c:2325)
> > rtnetlink_rcv_msg (net/core/rtnetlink.c:6959)
> > netlink_rcv_skb (net/netlink/af_netlink.c:2550)
> > </TASK>
> >
> > Fix this by substituting &noop_qdisc when new is NULL in
> > taprio_graft(), a common pattern used by other qdiscs (e.g.,
> > multiq_graft()) to ensure the q->qdiscs[] slots are never NULL.
> > This makes control-plane dump paths safe without requiring individual
> > NULL checks.
> >
> > Since the data-plane paths (taprio_enqueue and taprio_dequeue_from_txq)
> > previously had explicit NULL guards that would drop/skip the packet
> > cleanly, update those checks to test for &noop_qdisc instead. Without
> > this, packets would reach taprio_enqueue_one() which increments the root
> > qdisc's qlen and backlog before calling the child's enqueue; noop_qdisc
> > drops the packet but those counters are never rolled back, permanently
> > inflating the root qdisc's statistics.
> >
> > After this change *old can be a valid qdisc, NULL, or &noop_qdisc.
> > Only call qdisc_put(*old) in the first case to avoid decreasing
> > noop_qdisc's refcount, which was never increased.
> >
> > Fixes: 665338b2a7a0 ("net/sched: taprio: dump class stats for the actual q->qdiscs[]")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > Assisted-by: Claude:claude-opus-4-6
>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Please add Tested-by: if you tested it with the tdc patch in 2/2
>
> cheers,
> jamal

Tested-by: Weiming Shi <bestswngs@gmail.com>

Thanks,
Weiming Shi

> > ---
> > net/sched/sch_taprio.c | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > index 8e37528119506..a7daf34593e07 100644
> > --- a/net/sched/sch_taprio.c
> > +++ b/net/sched/sch_taprio.c
> > @@ -634,7 +634,7 @@ static int taprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> > queue = skb_get_queue_mapping(skb);
> >
> > child = q->qdiscs[queue];
> > - if (unlikely(!child))
> > + if (unlikely(child == &noop_qdisc))
> > return qdisc_drop(skb, sch, to_free);
> >
> > if (taprio_skb_exceeds_queue_max_sdu(sch, skb)) {
> > @@ -717,7 +717,7 @@ static struct sk_buff *taprio_dequeue_from_txq(struct Qdisc *sch, int txq,
> > int len;
> > u8 tc;
> >
> > - if (unlikely(!child))
> > + if (unlikely(child == &noop_qdisc))
> > return NULL;
> >
> > if (TXTIME_ASSIST_IS_ENABLED(q->flags))
> > @@ -2183,6 +2183,9 @@ static int taprio_graft(struct Qdisc *sch, unsigned long cl,
> > if (!dev_queue)
> > return -EINVAL;
> >
> > + if (!new)
> > + new = &noop_qdisc;
> > +
> > if (dev->flags & IFF_UP)
> > dev_deactivate(dev, false);
> >
> > @@ -2196,14 +2199,14 @@ static int taprio_graft(struct Qdisc *sch, unsigned long cl,
> > *old = q->qdiscs[cl - 1];
> > if (FULL_OFFLOAD_IS_ENABLED(q->flags)) {
> > WARN_ON_ONCE(dev_graft_qdisc(dev_queue, new) != *old);
> > - if (new)
> > + if (new != &noop_qdisc)
> > qdisc_refcount_inc(new);
> > - if (*old)
> > + if (*old && *old != &noop_qdisc)
> > qdisc_put(*old);
> > }
> >
> > q->qdiscs[cl - 1] = new;
> > - if (new)
> > + if (new != &noop_qdisc)
> > new->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
> >
> > if (dev->flags & IFF_UP)
> > --
> > 2.43.0
> >