Re: [PATCH 04/23] tick/nohz: Allow runtime changes in full dynticks CPUs

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Removing the TICK_DO_TIMER_NONE check in can_stop_idle_tick() allows all housekeeping CPUs to stop their ticks, permanently freezing timekeeping.
- [High] Modifying context_tracking.active for online CPUs bypasses RCU context tracking, causing Use-After-Free vulnerabilities.
- [High] Late context tracking initialization fails to propagate TIF_NOHZ to existing tasks and triggers an unconditional WARN_ON_ONCE.
- [Medium] Incomplete implementation: failed to remove the __init attribute from tick_nohz_init().
--

commit 4a1b2d82a8726c69f585b6c42f365c673f890897
Author: Waiman Long <longman@redhat.com>

tick/nohz: Allow runtime changes in full dynticks CPUs

This commit introduces tick_nohz_full_update_cpus() to update
tick_nohz_full_mask at runtime when isolated cpusets are changed.
It modifies context tracking to allow enabling and disabling user
context tracking dynamically.

>     We also need to enable CPU context tracking for those CPUs that
>     are in tick_nohz_full_mask. So remove __init from tick_nohz_init()
>     and ct_cpu_track_user() so that they be called later when an isolated
>     cpuset partition is being created.

This isn't a bug, but the commit message says __init is removed from
tick_nohz_init(), but this change doesn't appear in the patch diff.

> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 925999de1a28c..394e432630a39 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
[ ... ]
> @@ -674,9 +674,9 @@ void user_exit_callable(void)
>  }
>  NOKPROBE_SYMBOL(user_exit_callable);
>  
> -void __init ct_cpu_track_user(int cpu)
> +void ct_cpu_track_user(int cpu)
>  {
> -	static __initdata bool initialized = false;
> +	static bool initialized;

If this function runs for the first time during a runtime update, will it
trigger the WARN_ON_ONCE(!tasklist_empty()) located further down in this
function?

Also, since only the init_task receives the TIF_NOHZ flag during
this initialization block, will existing tasks that migrate to the newly
isolated CPUs be missing the flag and bypass context tracking checks?

> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 081f674a65882..67a747207f735 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
[ ... ]
> @@ -629,6 +626,36 @@ void __init tick_nohz_full_setup(cpumask_var_t cpumask)
>  	tick_nohz_full_running = true;
>  }
>  
> +/* Get the new set of run-time nohz CPU list & update accordingly */
> +void tick_nohz_full_update_cpus(struct cpumask *cpumask)
> +{
[ ... ]
> +	for_each_cpu_andnot(cpu, tick_nohz_full_mask, cpumask) {
> +		WARN_ON_ONCE(cpu_online(cpu));
> +		ct_cpu_untrack_user(cpu);
> +		cpumask_clear_cpu(cpu, tick_nohz_full_mask);
> +	}
> +}

Could this cause Use-After-Free issues if a CPU is online and executing in
userspace when its context tracking is disabled?

The WARN_ON_ONCE warns if the CPU is online, but doesn't prevent
ct_cpu_untrack_user() from running.

If the active flag is cleared while the CPU is in an RCU Extended Quiescent
State, won't it enter the kernel without calling rcu_user_exit(), causing
RCU to incorrectly assume the CPU is still in a quiescent state?

[ ... ]
> @@ -1238,10 +1265,6 @@ static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
>  		 */
>  		if (tick_cpu == cpu)
>  			return false;
> -
> -		/* Should not happen for nohz-full */
> -		if (WARN_ON_ONCE(tick_cpu == TICK_DO_TIMER_NONE))
> -			return false;
>  	}
>  
>  	return true;

Is it possible for timekeeping to permanently freeze here?

If tick_do_timer_cpu becomes TICK_DO_TIMER_NONE as mentioned in the commit
message, and all non-isolated housekeeping CPUs enter idle, won't they all
evaluate tick_cpu == cpu as false?

Without this check, it seems they will all proceed to stop their ticks,
leaving no CPU to take over the timekeeping duty.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=4