[PATCH net v2] net: sched: prevent scheduling deactivated qdiscs

[PATCH net v2] net: sched: prevent scheduling deactivated qdiscs

[David Lee]

From: Kyle Zeng <kylebot@openai.com>

TAPRIO can schedule a deactivated qdisc after some_qdisc_is_busy() has
reported it idle.

Commit 47e8dbb6e763 ("net/sched: do not reset queues in graft operations")
made graft deactivation skip queue reset, leaving TAPRIO's timer armed.

A late callback schedules NET_TX softirq work, whose dequeue path can
race the unlocked child-queue reset in taprio_reset(). The same skb can
be queued for deferred freeing by RTNL and dequeued for transmission.
In the reproducer, the TAP reader frees it before __rtnl_unlock() reads
its next pointer. KASAN reports a slab-use-after-free in
__rtnl_unlock+0xa3/0xd0, with a read of size 8 from the freed skb.

Reject deactivated qdiscs in __netif_schedule(). Callers' RCU protection
lets the existing grace period join earlier scheduling calls before the
busy check. Restart nonempty transmit queues on reactivation so suppressed
scheduling does not strand packets.

Fixes: 47e8dbb6e763 ("net/sched: do not reset queues in graft operations")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/20260805102514.740834-1-david.lee@trailofbits.com/
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Co-developed-by: David Lee <david.lee@trailofbits.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
---
Bug found and triaged by OpenAI Security Research and
validated by Trail of Bits.
Trail of Bits has a reproducer that triggers a KASAN use-after-free
and can share it if needed.


Changes in v2:
- Move the deactivation guard from TAPRIO into __netif_schedule().
- Correct Fixes to 47e8dbb6e763 and narrow the stable range to 7.1+.
- Retest with KASAN and check packet flow and temporary reactivation.

v1: https://lore.kernel.org/netdev/20260805102514.740834-1-david.lee@trailofbits.com/

 net/core/dev.c          | 3 +++
 net/sched/sch_generic.c | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a09e7869..bcd2d5197 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3478,6 +3478,9 @@ static void __netif_reschedule(struct Qdisc *q)
 
 void __netif_schedule(struct Qdisc *q)
 {
+	if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state)))
+		return;
+
 	/* If q->defer_list is not empty, at least one thread is
 	 * in __dev_xmit_skb() before llist_del_all(&q->defer_list).
 	 * This thread will attempt to run the queue.
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 6f6a6f0d5..bff425a14 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -1280,6 +1280,10 @@ static void transition_one_qdisc(struct net_device *dev,
 	if (need_watchdog_p) {
 		WRITE_ONCE(dev_queue->trans_start, jiffies);
 		*need_watchdog_p = 1;
+
+		/* Resume backlog left by a suppressed schedule while deactivated. */
+		if (!qdisc_is_empty(new_qdisc))
+			netif_schedule_queue(dev_queue);
 	}
 }
 

Re: [PATCH net v2] net: sched: prevent scheduling deactivated qdiscs

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch does not cover the IFF_UP-but-no-carrier case.

--

net: sched: prevent scheduling deactivated qdiscs

This patch makes __netif_schedule() return early for qdiscs that have
__QDISC_STATE_DEACTIVATED set. The goal is to stop a late TAPRIO
advance_timer callback from scheduling a qdisc after some_qdisc_is_busy()
has reported it idle. It also makes transition_one_qdisc() reschedule
nonempty queues on reactivation.

Link: https://lore.kernel.org/netdev/20260805102514.740834-1-david.lee@trailofbits.com/

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1a09e78691180..bcd2d5197d5b7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3478,6 +3478,9 @@ static void __netif_reschedule(struct Qdisc *q)
>  
>  void __netif_schedule(struct Qdisc *q)
>  {
> +	if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state)))
> +		return;
> +

[Severity: High]
Does this guard cover the case where the device is IFF_UP but the carrier
is off?

Only qdisc_deactivate() sets the DEACTIVATED bit, and
dev_deactivate_queue() calls it only on dev_queue->qdisc:

net/sched/sch_generic.c:dev_deactivate_queue() {
    ...
	qdisc = rtnl_dereference(dev_queue->qdisc);
	if (qdisc) {
		if (qdisc->enqueue)
			*sync_needed = true;
		qdisc_deactivate(qdisc);
		rcu_assign_pointer(dev_queue->qdisc, &noop_qdisc);
	}
}

With the carrier off, dev_activate() returns before it reaches
transition_one_qdisc():

net/sched/sch_generic.c:dev_activate() {
    ...
	if (!netif_carrier_ok(dev))
		/* Delay activation until next carrier-on event */
		return;
    ...
}

So dev_queue->qdisc stays &noop_qdisc. Because noop_qdisc is
TCQ_F_BUILTIN, qdisc_deactivate() does nothing to it. The TAPRIO in
qdisc_sleeping never gets the bit, and qdisc_alloc() does not set it
either.

Here is a possible sequence, for example on a multiqueue veth whose peer
is down, or a multiqueue TAP with no reader:

1. tc qdisc add ... root taprio (software mode). taprio_change() calls
   taprio_start_sched(), which arms advance_timer. taprio_attach() puts
   the taprio in each qdisc_sleeping, and dev_activate() returns early.

2. tc qdisc del/replace root. qdisc_graft() calls
   dev_deactivate(dev, false). Only noop_qdisc is flagged.
   dev_reset_queue() is skipped because reset_needed is false, and
   some_qdisc_is_busy() returns. advance_timer is still armed.

3. On another CPU:

   advance_sched()
     __netif_schedule(sch)    <- DEACTIVATED is clear, SCHED gets set
   net_tx_action()
     clear_bit(__QDISC_STATE_SCHED, &q->state)
     qdisc_run()
       taprio_dequeue()
         taprio_dequeue_from_txq()
           child = q->qdiscs[txq]

4. Meanwhile, on CPU0:

   notify_and_destroy()
     qdisc_put()
       __qdisc_destroy()
         qdisc_reset()        <- taprio_reset(), without the root lock
         taprio_destroy()
           hrtimer_cancel(&q->advance_timer)
           qdisc_synchronize(sch)
           kfree(q->qdiscs)
           q->qdiscs = NULL

qdisc_synchronize() waits only for SCHED to clear. It does not wait for a
qdisc_run() that has already started, and net_tx_action() clears SCHED
before it calls qdisc_run().

Can taprio_dequeue_from_txq() then read the freed q->qdiscs array and call
child->ops->peek() through the pointer it loaded? And if it runs after
q->qdiscs = NULL, does it hit a NULL dereference instead?

The skb double-ownership in the commit message is probably not possible
here. Enqueues go to noop_qdisc, so the child queues are empty. It is
still the same race, though: TAPRIO gets scheduled after the busy check.

Before 47e8dbb6e763, graft-time dev_deactivate() always ran
dev_reset_queue() on qdisc_sleeping. That reached taprio_reset() and
hrtimer_cancel() before some_qdisc_is_busy(), whatever the carrier state.

Would it make sense to stop or neutralize the TAPRIO advance_timer for the
sleeping qdisc during graft deactivation? For example, the timer could be
cancelled before the busy wait, or qdisc_sleeping could be flagged as well
as dev_queue->qdisc.

This next part is a pre-existing issue, not one introduced by this patch.
If the device is not IFF_UP, qdisc_graft() skips dev_deactivate()
entirely:

net/sched/sch_api.c:qdisc_graft() {
    ...
		if (dev->flags & IFF_UP)
			dev_deactivate(dev, false);
    ...
}

So a taprio created on a down interface can still be scheduled by its own
timer while it is being destroyed.

>  	/* If q->defer_list is not empty, at least one thread is
>  	 * in __dev_xmit_skb() before llist_del_all(&q->defer_list).
>  	 * This thread will attempt to run the queue.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261023194712.package1-20-v2-david.lee%40trailofbits.com

Re: [PATCH net v2] net: sched: prevent scheduling deactivated qdiscs

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Dave Miller <davem@davem-thinkpadp1gen8.ibmlond.csb>:

On Fri, 23 Oct 2026 16:02:56 -0400 you wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> TAPRIO can schedule a deactivated qdisc after some_qdisc_is_busy() has
> reported it idle.
> 
> Commit 47e8dbb6e763 ("net/sched: do not reset queues in graft operations")
> made graft deactivation skip queue reset, leaving TAPRIO's timer armed.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: sched: prevent scheduling deactivated qdiscs
    https://git.kernel.org/netdev/net/c/d6f81fd6abe2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html