[PATCH net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

[PATCH net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

[Vinicius Costa Gomes]

In advance_sched(), when should_change_schedules() returns true,
switch_schedules() is called to promote the admin schedule to oper.
switch_schedules() queues the old oper schedule for RCU freeing via
call_rcu(), but 'next' still points into an entry of the old oper
schedule. The subsequent 'next->end_time = end_time' and
rcu_assign_pointer(q->current_entry, next) are use-after-free.

Fix this by selecting 'next' from the new oper schedule immediately
after switch_schedules(), and using its pre-calculated end_time.
setup_first_end_time() sets the first entry's end_time to
base_time + interval when the schedule is installed, so the value
is already correct.

The deleted 'end_time = sched_base_time(admin)' assignment was also
harmful independently: it would overwrite the new first entry's
pre-calculated end_time with just base_time.

Fixes: a3d43c0d56f1 ("taprio: Add support adding an admin schedule")
Reported-by: Junxi Qian <qjx1298677004@gmail.com>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 net/sched/sch_taprio.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index f721c03514f6..0316f2dee06a 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -972,11 +972,12 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)
 	}
 
 	if (should_change_schedules(admin, oper, end_time)) {
-		/* Set things so the next time this runs, the new
-		 * schedule runs.
-		 */
-		end_time = sched_base_time(admin);
 		switch_schedules(q, &admin, &oper);
+		/* After changing schedules, the next entry is the first one
+		 * in the new schedule, with a pre-calculated end_time.
+		 */
+		next = list_first_entry(&oper->entries, struct sched_entry, list);
+		end_time = next->end_time;
 	}
 
 	next->end_time = end_time;

---
base-commit: 02f72964395911e7a09bb2ea2fe6f79eda4ea2c2
change-id: 20260410-taprio-user-after-free-fix-net-fbd67f66512b

Best regards,
--  
Vinicius

Re: [PATCH net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

This commit updates advance_sched() to select the next entry from the new
oper schedule immediately after switch_schedules() returns. It relies on the
pre-calculated end_time of the new schedule's first entry instead of
overwriting it with the admin schedule's base time.

> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index f721c03514f60..0316f2dee06ac 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c

[ ... ]

> @@ -972,11 +972,12 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)
>  	}
>  
>  	if (should_change_schedules(admin, oper, end_time)) {
> -		/* Set things so the next time this runs, the new
> -		 * schedule runs.
> -		 */
> -		end_time = sched_base_time(admin);

Does removing this assignment introduce a regression where the new schedule
activates prematurely?

Since the new schedule's first entry has its end_time pre-calculated
as base_time + interval, if we activate it early (before its actual
base_time), does it stretch the entry backwards in time?

Could this cause its gate mask to apply during the time gap, violating the
strictly configured gate durations?

>  		switch_schedules(q, &admin, &oper);

Can this sequence cause a use-after-free regression due to the RCU grace
period starting too early?

Inside switch_schedules(), call_rcu() is invoked on the old oper
schedule. However, q->current_entry is not updated until later in
advance_sched() at the first_run label:

first_run:
	rcu_assign_pointer(q->current_entry, next);

If a concurrent reader calls rcu_dereference(q->current_entry) between
switch_schedules() and this assignment, can it acquire a pointer to the
old schedule after call_rcu() has already started? This would mean the
grace period will not wait for this new reader.

As a separate pre-existing issue, can switch_schedules() be called
concurrently by lockless readers?

If advance_sched() calls it under q->current_entry_lock, but a concurrent
reader like get_packet_txtime() evaluates the switch condition and calls
switch_schedules() without holding locks, can they race?

Could this lead to call_rcu() being called twice on the same old oper
object, potentially corrupting the RCU callback list?

> +		/* After changing schedules, the next entry is the first one
> +		 * in the new schedule, with a pre-calculated end_time.
> +		 */
> +		next = list_first_entry(&oper->entries, struct sched_entry, list);
> +		end_time = next->end_time;
>  	}
>  
>  	next->end_time = end_time;

Re: [PATCH net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

[Vinicius Costa Gomes]

Jakub Kicinski <kuba@kernel.org> writes:

> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net/sched: taprio: fix use-after-free in advance_sched() on schedule switch
>
> This commit updates advance_sched() to select the next entry from the new
> oper schedule immediately after switch_schedules() returns. It relies on the
> pre-calculated end_time of the new schedule's first entry instead of
> overwriting it with the admin schedule's base time.
>
>> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
>> index f721c03514f60..0316f2dee06ac 100644
>> --- a/net/sched/sch_taprio.c
>> +++ b/net/sched/sch_taprio.c
>
> [ ... ]
>
>> @@ -972,11 +972,12 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)
>>  	}
>>  
>>  	if (should_change_schedules(admin, oper, end_time)) {
>> -		/* Set things so the next time this runs, the new
>> -		 * schedule runs.
>> -		 */
>> -		end_time = sched_base_time(admin);
>
> Does removing this assignment introduce a regression where the new schedule
> activates prematurely?
>

should_change_schedules() checks if the previous 'end_time' is after the
start of the "new" admin schedule.

> Since the new schedule's first entry has its end_time pre-calculated
> as base_time + interval, if we activate it early (before its actual
> base_time), does it stretch the entry backwards in time?

If the hrtimer triggers early, that might be possible. But one fact is
that this is on the software implementation, and as its accuracy is the
hrtimer accuracy, second guessing what the hrtimer did/is doing here
seems wrong.

>
> Could this cause its gate mask to apply during the time gap, violating the
> strictly configured gate durations?

Yeah, if the hrtimer fires early, we might have some schedule entries
that are longer than they should be. But that is an intrinsic limitation
of the software mode (at least in its current implementation). 

So, in short, not really concerned about this "violation of strictness",
as long as we are not making it worse, to be clear.

>
>>  		switch_schedules(q, &admin, &oper);
>
> Can this sequence cause a use-after-free regression due to the RCU grace
> period starting too early?
>
> Inside switch_schedules(), call_rcu() is invoked on the old oper
> schedule. However, q->current_entry is not updated until later in
> advance_sched() at the first_run label:
>
> first_run:
> 	rcu_assign_pointer(q->current_entry, next);
>
> If a concurrent reader calls rcu_dereference(q->current_entry) between
> switch_schedules() and this assignment, can it acquire a pointer to the
> old schedule after call_rcu() has already started? This would mean the
> grace period will not wait for this new reader.

I had to sleep on this, yeah, this is possible, but the window is very
narrow. And not introduced by this fix. I guess this problem existed
since I added the concept of an "admin" schedule.

Will fix on a separate patch. Need to think more about this.

>
> As a separate pre-existing issue, can switch_schedules() be called
> concurrently by lockless readers?
>
> If advance_sched() calls it under q->current_entry_lock, but a concurrent
> reader like get_packet_txtime() evaluates the switch condition and calls
> switch_schedules() without holding locks, can they race?

At configuration time, txtime-assisted (which calls get_packet_txtime())
mode and the software mode (which "runs" advance_sched()) are blocked to
run at the same time. This should not be possible.

>
> Could this lead to call_rcu() being called twice on the same old oper
> object, potentially corrupting the RCU callback list?
>
>> +		/* After changing schedules, the next entry is the first one
>> +		 * in the new schedule, with a pre-calculated end_time.
>> +		 */
>> +		next = list_first_entry(&oper->entries, struct sched_entry, list);
>> +		end_time = next->end_time;
>>  	}
>>  
>>  	next->end_time = end_time;


Cheers,
-- 
Vinicius

Re: [PATCH net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 10 Apr 2026 18:57:57 -0700 you wrote:
> In advance_sched(), when should_change_schedules() returns true,
> switch_schedules() is called to promote the admin schedule to oper.
> switch_schedules() queues the old oper schedule for RCU freeing via
> call_rcu(), but 'next' still points into an entry of the old oper
> schedule. The subsequent 'next->end_time = end_time' and
> rcu_assign_pointer(q->current_entry, next) are use-after-free.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: taprio: fix use-after-free in advance_sched() on schedule switch
    https://git.kernel.org/netdev/net/c/105425b1969c

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