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

[Paolo Abeni <pabeni@redhat.com>]

On 3/30/26 12:29 PM, Weiming Shi 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:2475)
>  Call Trace:
>   <TASK>
>   tc_fill_tclass (net/sched/sch_api.c:1966)
>   qdisc_class_dump (net/sched/sch_api.c:2329)
>   taprio_walk (net/sched/sch_taprio.c:2510)
>   tc_dump_tclass_qdisc (net/sched/sch_api.c:2353)
>   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:6827)
>   netlink_dump (net/netlink/af_netlink.c:2325)
>   rtnetlink_rcv_msg (net/core/rtnetlink.c:6927)
>   netlink_rcv_skb (net/netlink/af_netlink.c:2550)
>   </TASK>
> 
> Fix this by substituting &noop_qdisc when new is NULL in
> taprio_graft(), following the same pattern used by multiq_graft() and
> prio_graft(). This ensures q->qdiscs[] slots are never NULL, making
> all consumer paths (dump, enqueue, dequeue) safe. The noop_qdisc is a
> kernel-global builtin qdisc that drops all packets, which is
> functionally equivalent to a NULL child for data path purposes. The
> refcount increment and flag modification are guarded with
> != &noop_qdisc to avoid modifying the global singleton.
> 
> Fixes: 665338b2a7a0 ("net/sched: taprio: dump class stats for the actual q->qdiscs[]")
> Cc: stable@vger.kernel.org
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  net/sched/sch_taprio.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index f721c03514f60..cecaef16c0dd1 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -2183,6 +2183,9 @@ static int taprio_graft(struct Qdisc *sch, unsigned long cl,
>  	if (!dev_queue)
>  		return -EINVAL;
>  
> +	if (!new)
> +		new = &noop_qdisc;

Sashiko says:

---
Does replacing a deleted child with &noop_qdisc instead of NULL leak the
qlen and backlog counters of the root taprio qdisc?
Before this change, taprio_enqueue() checked for a NULL child and dropped
the packet safely without incrementing the root qdisc's stats. With the
child now set to &noop_qdisc, the check passes and execution proceeds to
taprio_enqueue_one().
Inside taprio_enqueue_one(), the root qdisc's qlen and backlog are
incremented before calling the child's enqueue function:
taprio_enqueue_one() {
    ...
    sch->q.qlen++;
    sch->qstats.backlog += qdisc_pkt_len(skb);
    ...
    qdisc_enqueue(skb, child, to_free);
}
When the child is noop_qdisc, its enqueue callback drops the packet and
returns a drop status. However, taprio_enqueue_one() does not appear to
roll back the incremented qlen and backlog counters to account for the
drop.
Can this permanently inflate the root qdisc's queue length and backlog,
breaking statistics and disabling transmission fast-path bypasses?
---

It looks like you additionally need to replace NULL checks for
q->qdiscs[] with check vs '&noop_qdisc' in many places.

/P