mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	tracing/timerlat: Add interface_lock around clearing of kthread in stop_kthread()
The timerlat interface will get and put the task that is part of the
"kthread" field of the osn_var to keep it around until all references are
released. But here's a race in the "stop_kthread()" code that will call
put_task_struct() on the kthread if it is not a kernel thread. This can
race with the releasing of the references to that task struct and the
put_task_struct() can be called twice when it should have been called just
once.
Take the interface_lock() in stop_kthread() to synchronize this change.
But to do so, the function stop_per_cpu_kthreads() needs to change the
loop from for_each_online_cpu() to for_each_possible_cpu() and remove the
cpu_read_lock(), as the interface_lock can not be taken while the cpu
locks are held. The only side effect of this change is that it may do some
extra work, as the per_cpu variables of the offline CPUs would not be set
anyway, and would simply be skipped in the loop.
Remove unneeded "return;" in stop_kthread().
Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Tomas Glozar <tglozar@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: "Luis Claudio R. Goncalves" <lgoncalv@redhat.com>
Link: https://lore.kernel.org/20240905113359.2b934242@gandalf.local.home
Fixes: e88ed227f6 ("tracing/timerlat: Add user-space interface")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
			
			
This commit is contained in:
		
							parent
							
								
									e6a53481da
								
							
						
					
					
						commit
						5bfbcd1ee5
					
				
					 1 changed files with 6 additions and 7 deletions
				
			
		| 
						 | 
				
			
			@ -1953,8 +1953,12 @@ static void stop_kthread(unsigned int cpu)
 | 
			
		|||
{
 | 
			
		||||
	struct task_struct *kthread;
 | 
			
		||||
 | 
			
		||||
	mutex_lock(&interface_lock);
 | 
			
		||||
	kthread = per_cpu(per_cpu_osnoise_var, cpu).kthread;
 | 
			
		||||
	if (kthread) {
 | 
			
		||||
		per_cpu(per_cpu_osnoise_var, cpu).kthread = NULL;
 | 
			
		||||
		mutex_unlock(&interface_lock);
 | 
			
		||||
 | 
			
		||||
		if (cpumask_test_and_clear_cpu(cpu, &kthread_cpumask) &&
 | 
			
		||||
		    !WARN_ON(!test_bit(OSN_WORKLOAD, &osnoise_options))) {
 | 
			
		||||
			kthread_stop(kthread);
 | 
			
		||||
| 
						 | 
				
			
			@ -1967,8 +1971,8 @@ static void stop_kthread(unsigned int cpu)
 | 
			
		|||
			kill_pid(kthread->thread_pid, SIGKILL, 1);
 | 
			
		||||
			put_task_struct(kthread);
 | 
			
		||||
		}
 | 
			
		||||
		per_cpu(per_cpu_osnoise_var, cpu).kthread = NULL;
 | 
			
		||||
	} else {
 | 
			
		||||
		mutex_unlock(&interface_lock);
 | 
			
		||||
		/* if no workload, just return */
 | 
			
		||||
		if (!test_bit(OSN_WORKLOAD, &osnoise_options)) {
 | 
			
		||||
			/*
 | 
			
		||||
| 
						 | 
				
			
			@ -1976,7 +1980,6 @@ static void stop_kthread(unsigned int cpu)
 | 
			
		|||
			 */
 | 
			
		||||
			per_cpu(per_cpu_osnoise_var, cpu).sampling = false;
 | 
			
		||||
			barrier();
 | 
			
		||||
			return;
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -1991,12 +1994,8 @@ static void stop_per_cpu_kthreads(void)
 | 
			
		|||
{
 | 
			
		||||
	int cpu;
 | 
			
		||||
 | 
			
		||||
	cpus_read_lock();
 | 
			
		||||
 | 
			
		||||
	for_each_online_cpu(cpu)
 | 
			
		||||
	for_each_possible_cpu(cpu)
 | 
			
		||||
		stop_kthread(cpu);
 | 
			
		||||
 | 
			
		||||
	cpus_read_unlock();
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/*
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue