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

[Paolo Abeni <pabeni@redhat.com>]

On 4/10/26 5:39 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
> control-plane dump paths safe without requiring individual NULL checks.
> 
> Also update the data-plane NULL guards in taprio_enqueue() and
> taprio_dequeue_from_txq() to check for &noop_qdisc, so that packets
> are still dropped cleanly without inflating qlen/backlog counters.
> 
> 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>
> ---
> v2:
>   - Update NULL checks in taprio_enqueue() and taprio_dequeue_from_txq()
>     to test for &noop_qdisc instead of NULL, preventing qlen/backlog
>     counter inflation when noop_qdisc drops packets (Sashiko)
> ---
>  net/sched/sch_taprio.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index f721c03514f60..XXXXXXXXX 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,
> 
>  	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 prio;
>  	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);
> 
> @@ -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)
>  			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

Does not apply cleanly to net and looks seriously mangled. I suspect the
above chunks should be part of the actual patch ?!?

Please, whatever tool you are using to help crafting the patch, double
check the result manually before the actual submission.

/P