Re: [PATCH net] net/sched: sch_taprio: do not requeue a deactivated qdisc

On Wed, 05 Aug 2026 13:19:36 -0700 Vinicius Costa Gomes wrote:

David Lee [off-list ref] writes:

quoted

From: Kyle Zeng <redacted>

Root qdisc replacement and deletion call dev_deactivate() without
resetting the old qdisc. This marks the qdisc deactivated and waits for
existing runs to finish, but leaves TAPRIO's private hrtimer active.
advance_sched() can therefore requeue the old root after the final busy
check, allowing a new run to overlap reset and destruction.

Do not schedule TAPRIO after its root has been deactivated. Keep the
test in the existing RCU read-side critical section so that it pairs
with the synchronize_net() in dev_deactivate_many(): a callback which
observes an active qdisc must finish before the final busy check, while
a later callback observes the deactivated state and skips the requeue.

Fixes: 5a781ccbd19e ("tc: Add support for configuring the taprio scheduler")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
Signed-off-by: Kyle Zeng <redacted>
Co-developed-by: David Lee <redacted>
Signed-off-by: David Lee <redacted>
---
Bug found and triaged by OpenAI Security Research and
validated by Trail of Bits.

The supplied v7.2-rc3 trace contains a KASAN use-after-free. The
reproducer did not trigger a sanitizer report in the current v7.2-rc5
campaign and can be shared if needed.

 net/sched/sch_taprio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 299234a5f..2cf76df43 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c

@@ -990,7 +990,8 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)
 	hrtimer_set_expires(&q->advance_timer, end_time);
 
 	rcu_read_lock();
-	__netif_schedule(sch);
+	if (!test_bit(__QDISC_STATE_DEACTIVATED, &sch->state))
+		__netif_schedule(sch);
 	rcu_read_unlock();
 

I'll be the first one to admit that taprio is a weird one (that it keeps
a timer around while it's running among others), but it looks to me that
this check would make more sense inside __netif_schedule().

Let's see what others think.

see the clashiko AI comment below. If that's true and indeed problem
did not exist before 47e8dbb6e763e5 -- then the fix is misplaced,
like you say. (I'm not sure about __netif_schedule(), to be clear,
but some_qdisc_is_busy() is not strong enough?)


The changelog opens with:
  "Root qdisc replacement and deletion call dev_deactivate() without
   resetting the old qdisc."
Is that true for the trees the Fixes: tag points at?
That behaviour looks like it arrives with 47e8dbb6e763e5 ("net/sched: do
not reset queues in graft operations"), which added the reset_needed
argument and made qdisc_graft() use:
net/sched/sch_api.c:qdisc_graft() {
	...
		if (dev->flags & IFF_UP)
			dev_deactivate(dev, false);
	...
}
Before that, dev_deactivate_many() ran dev_reset_queue() on every txq
unconditionally, and it did so before the some_qdisc_is_busy() wait loop.
dev_reset_queue() resets rtnl_dereference(dev_queue->qdisc_sleeping), which
in the root-graft path is still the old taprio qdisc, so
qdisc_reset() -> taprio_reset() -> hrtimer_cancel() disarmed advance_timer
before the busy check, and nothing re-arms it outside
taprio_change()/taprio_start_sched() under RTNL.
If that reading is right, the requeue-after-busy-check window does not
exist without 47e8dbb6e763e5, but the patch carries
Fixes: 5a781ccbd19e ("tc: Add support for configuring the taprio
scheduler") plus Cc: stable, which aims it at every stable tree back to
v4.20.
Should the Fixes: tag name the commit that made dev_deactivate() skip the
reset, and should the changelog mention that this reset-skipping behaviour
is a recent change, so the backport range is clear?
-- 
pw-bot: cr