mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
@ 2026-08-28 18:03 Waiman Long
  2026-08-31  1:50 ` Ridong Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Waiman Long @ 2026-08-28 18:03 UTC (permalink / raw)
  To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný
  Cc: cgroups, linux-kernel, Farhad Alemi, Waiman Long

With commit 2125c0034c5d ("cgroup/cpuset: Make cpuset hotplug
processing synchronous"), the cpuset hotplug operation becomes
synchronous. That commit also removes the code that handles the race
between cpuset_hotplug_work and cpu hotplug notifier with the assumption
that race is now gone. Later commit 7a0aabd9ce69 ("cgroup/cpuset:
Always use cpu_active_mask") updates the cpuset code to always use
cpu_active_mask instead of cpu_ohline_mask in various places including
guarantee_online_cpus() which is also renamed to guarantee_active_cpus()
in that commit.

In the case of CPU offline operation, cpuset_active_mask is updated
first in sched_cpu_deactivate() to remove the offline CPU before
cpuset_handle_hotplug() is called to update the effective_cpus of the
affected cpusets. The cpu_online_mask is updated after that near the end
of the offline operation to remove the offline CPU. As a result, the race
comes back and the top cpuset may not have any active CPU leading to NULL
pointer dereference during the race window when guarantee_active_cpus()
is called after cpu_active_mask is updated to remove the CPU to be torn
down but before cpuset_handle_hotplug() is able to properly update the
effective_cpus of the top cpuset.

Fix this by adding back the NULL cs check to avoid this problem. However,
the output of guarantee_active_cpus() will be unreliable during that
race window.

Fixes: 7a0aabd9ce69 ("cgroup/cpuset: Always use cpu_active_mask")
Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Link: https://lore.kernel.org/lkml/CA+0ovChh3VjsKN1g+ZGjwwY2fGTpP7uD+aCCByLj5Qbymw=bfQ@mail.gmail.com
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0bad5bbec6bf..309478ab21ad 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -513,10 +513,26 @@ static void guarantee_active_cpus(struct task_struct *tsk,
 	rcu_read_lock();
 	cs = task_cs(tsk);
 
-	while (!cpumask_intersects(cs->effective_cpus, pmask))
+	while (!cpumask_intersects(cs->effective_cpus, pmask)) {
 		cs = parent_cs(cs);
-
+		if (unlikely(!cs)) {
+			/*
+			 * The top cpuset doesn't have any active cpu as a
+			 * consequence of a race between its caller and the cpu
+			 * hotplug operation where cpu_active_mask is updated
+			 * asynchronously before cpuset_handle_hotplug() is
+			 * being called to adjust the effective_cpus of the
+			 * affected cpusets. But we know the top cpuset's
+			 * effective_cpus is on its way to be identical to
+			 * cpu_active_mask minus the exclusive CPUs dedicated
+			 * to other valid cpuset partitions. Just pass back
+			 * the filtered cpu_active_mask in this case.
+			 */
+			goto out_unlock;
+		}
+	}
 	cpumask_and(pmask, pmask, cs->effective_cpus);
+out_unlock:
 	rcu_read_unlock();
 }
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
  2026-08-28 18:03 [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus() Waiman Long
@ 2026-08-31  1:50 ` Ridong Chen
  2026-09-01  0:08   ` Waiman Long
  0 siblings, 1 reply; 6+ messages in thread
From: Ridong Chen @ 2026-08-31  1:50 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný
  Cc: cgroups, linux-kernel, Farhad Alemi



On 8/29/2026 2:03 AM, Waiman Long wrote:
> With commit 2125c0034c5d ("cgroup/cpuset: Make cpuset hotplug
> processing synchronous"), the cpuset hotplug operation becomes
> synchronous. That commit also removes the code that handles the race
> between cpuset_hotplug_work and cpu hotplug notifier with the assumption
> that race is now gone. Later commit 7a0aabd9ce69 ("cgroup/cpuset:
> Always use cpu_active_mask") updates the cpuset code to always use
> cpu_active_mask instead of cpu_ohline_mask in various places including
> guarantee_online_cpus() which is also renamed to guarantee_active_cpus()
> in that commit.
> 
> In the case of CPU offline operation, cpuset_active_mask is updated
> first in sched_cpu_deactivate() to remove the offline CPU before
> cpuset_handle_hotplug() is called to update the effective_cpus of the
> affected cpusets. The cpu_online_mask is updated after that near the end
> of the offline operation to remove the offline CPU. As a result, the race
> comes back and the top cpuset may not have any active CPU leading to NULL
> pointer dereference during the race window when guarantee_active_cpus()
> is called after cpu_active_mask is updated to remove the CPU to be torn
> down but before cpuset_handle_hotplug() is able to properly update the
> effective_cpus of the top cpuset.
> 

Hi Longman,

Thank you for the patch. I believe it should resolve the issue.
Could Farhad please verify whether it works as expected?

> Fix this by adding back the NULL cs check to avoid this problem. However,
> the output of guarantee_active_cpus() will be unreliable during that
> race window.
> 

If guarantee_active_cpus() can return unreliable results during that race 
window, should it instead return an error when it cannot guarantee that the 
active CPUs are usable? Otherwise, callers might use an incorrect 'pmask', which 
could lead to other issues downstream.

> Fixes: 7a0aabd9ce69 ("cgroup/cpuset: Always use cpu_active_mask")
> Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
> Link: https://lore.kernel.org/lkml/CA+0ovChh3VjsKN1g+ZGjwwY2fGTpP7uD+aCCByLj5Qbymw=bfQ@mail.gmail.com
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   kernel/cgroup/cpuset.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0bad5bbec6bf..309478ab21ad 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -513,10 +513,26 @@ static void guarantee_active_cpus(struct task_struct *tsk,
>   	rcu_read_lock();
>   	cs = task_cs(tsk);
>   
> -	while (!cpumask_intersects(cs->effective_cpus, pmask))
> +	while (!cpumask_intersects(cs->effective_cpus, pmask)) {
>   		cs = parent_cs(cs);
> -
> +		if (unlikely(!cs)) {
> +			/*
> +			 * The top cpuset doesn't have any active cpu as a
> +			 * consequence of a race between its caller and the cpu
> +			 * hotplug operation where cpu_active_mask is updated
> +			 * asynchronously before cpuset_handle_hotplug() is
> +			 * being called to adjust the effective_cpus of the
> +			 * affected cpusets. But we know the top cpuset's
> +			 * effective_cpus is on its way to be identical to
> +			 * cpu_active_mask minus the exclusive CPUs dedicated
> +			 * to other valid cpuset partitions. Just pass back
> +			 * the filtered cpu_active_mask in this case.
> +			 */
> +			goto out_unlock;
> +		}
> +	}
>   	cpumask_and(pmask, pmask, cs->effective_cpus);
> +out_unlock:
>   	rcu_read_unlock();
>   }
>   

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
  2026-08-31  1:50 ` Ridong Chen
@ 2026-09-01  0:08   ` Waiman Long
  2026-09-01 20:55     ` Farhad Alemi
  0 siblings, 1 reply; 6+ messages in thread
From: Waiman Long @ 2026-09-01  0:08 UTC (permalink / raw)
  To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný
  Cc: cgroups, linux-kernel, Farhad Alemi

On 8/30/26 9:50 PM, Ridong Chen wrote:
>
>
> On 8/29/2026 2:03 AM, Waiman Long wrote:
>> With commit 2125c0034c5d ("cgroup/cpuset: Make cpuset hotplug
>> processing synchronous"), the cpuset hotplug operation becomes
>> synchronous. That commit also removes the code that handles the race
>> between cpuset_hotplug_work and cpu hotplug notifier with the assumption
>> that race is now gone. Later commit 7a0aabd9ce69 ("cgroup/cpuset:
>> Always use cpu_active_mask") updates the cpuset code to always use
>> cpu_active_mask instead of cpu_ohline_mask in various places including
>> guarantee_online_cpus() which is also renamed to guarantee_active_cpus()
>> in that commit.
>>
>> In the case of CPU offline operation, cpuset_active_mask is updated
>> first in sched_cpu_deactivate() to remove the offline CPU before
>> cpuset_handle_hotplug() is called to update the effective_cpus of the
>> affected cpusets. The cpu_online_mask is updated after that near the end
>> of the offline operation to remove the offline CPU. As a result, the 
>> race
>> comes back and the top cpuset may not have any active CPU leading to 
>> NULL
>> pointer dereference during the race window when guarantee_active_cpus()
>> is called after cpu_active_mask is updated to remove the CPU to be torn
>> down but before cpuset_handle_hotplug() is able to properly update the
>> effective_cpus of the top cpuset.
>>
>
> Hi Longman,
>
> Thank you for the patch. I believe it should resolve the issue.
> Could Farhad please verify whether it works as expected?
>
>> Fix this by adding back the NULL cs check to avoid this problem. 
>> However,
>> the output of guarantee_active_cpus() will be unreliable during that
>> race window.
>>
>
> If guarantee_active_cpus() can return unreliable results during that 
> race window, should it instead return an error when it cannot 
> guarantee that the active CPUs are usable? Otherwise, callers might 
> use an incorrect 'pmask', which could lead to other issues downstream.

I am thinking about that too. However, the change will be more extensive 
and it is that easy to figure out if we are the in the race window. So 
more thought will be needed before we can make that change. In the mean 
time, we have to fix the NULL pointer dereference and it has behaved 
like that before.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
  2026-09-01  0:08   ` Waiman Long
@ 2026-09-01 20:55     ` Farhad Alemi
  2026-09-03  3:29       ` Waiman Long
  0 siblings, 1 reply; 6+ messages in thread
From: Farhad Alemi @ 2026-09-01 20:55 UTC (permalink / raw)
  To: Waiman Long
  Cc: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	cgroups, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2721 bytes --]

Apologies for the delay in response. Upon our run, the patched kernel
388b607d107c07aaade04c7f22f344cab6bdccd3 no longer panics when facing
the attached reproducer.

Thanks!

On Mon, Aug 31, 2026 at 5:09 PM Waiman Long <longman@redhat.com> wrote:
>
> On 8/30/26 9:50 PM, Ridong Chen wrote:
> >
> >
> > On 8/29/2026 2:03 AM, Waiman Long wrote:
> >> With commit 2125c0034c5d ("cgroup/cpuset: Make cpuset hotplug
> >> processing synchronous"), the cpuset hotplug operation becomes
> >> synchronous. That commit also removes the code that handles the race
> >> between cpuset_hotplug_work and cpu hotplug notifier with the assumption
> >> that race is now gone. Later commit 7a0aabd9ce69 ("cgroup/cpuset:
> >> Always use cpu_active_mask") updates the cpuset code to always use
> >> cpu_active_mask instead of cpu_ohline_mask in various places including
> >> guarantee_online_cpus() which is also renamed to guarantee_active_cpus()
> >> in that commit.
> >>
> >> In the case of CPU offline operation, cpuset_active_mask is updated
> >> first in sched_cpu_deactivate() to remove the offline CPU before
> >> cpuset_handle_hotplug() is called to update the effective_cpus of the
> >> affected cpusets. The cpu_online_mask is updated after that near the end
> >> of the offline operation to remove the offline CPU. As a result, the
> >> race
> >> comes back and the top cpuset may not have any active CPU leading to
> >> NULL
> >> pointer dereference during the race window when guarantee_active_cpus()
> >> is called after cpu_active_mask is updated to remove the CPU to be torn
> >> down but before cpuset_handle_hotplug() is able to properly update the
> >> effective_cpus of the top cpuset.
> >>
> >
> > Hi Longman,
> >
> > Thank you for the patch. I believe it should resolve the issue.
> > Could Farhad please verify whether it works as expected?
> >
> >> Fix this by adding back the NULL cs check to avoid this problem.
> >> However,
> >> the output of guarantee_active_cpus() will be unreliable during that
> >> race window.
> >>
> >
> > If guarantee_active_cpus() can return unreliable results during that
> > race window, should it instead return an error when it cannot
> > guarantee that the active CPUs are usable? Otherwise, callers might
> > use an incorrect 'pmask', which could lead to other issues downstream.
>
> I am thinking about that too. However, the change will be more extensive
> and it is that easy to figure out if we are the in the race window. So
> more thought will be needed before we can make that change. In the mean
> time, we have to fix the NULL pointer dereference and it has behaved
> like that before.
>
> Cheers,
> Longman
>

[-- Attachment #2: reproducer.c --]
[-- Type: application/octet-stream, Size: 1547 bytes --]

/*
 * Reproducer for 166-general-protection-fault-in-cpuset-cpus-allowed.
 *
 * Build: gcc -pthread -static -O0 -w -o repro reproducer.c
 * Run:   as root in a disposable VM.
 */
#define _GNU_SOURCE
#include <fcntl.h>
#include <pthread.h>
#include <sched.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>
#include <unistd.h>

#define CG "/sys/fs/cgroup"

static pid_t victim;

static void wr(const char *path, const char *val)
{
	int fd = open(path, O_WRONLY);

	if (fd < 0)
		return;
	write(fd, val, strlen(val));
	close(fd);
}

static void cg_wr(const char *grp, const char *file, const char *val)
{
	char path[128];

	snprintf(path, sizeof(path), CG "/%s/%s", grp, file);
	wr(path, val);
}

static void *cpu1_flap(void *unused)
{
	for (;;) {
		wr("/sys/devices/system/cpu/cpu1/online", "0");
		wr("/sys/devices/system/cpu/cpu1/online", "1");
		cg_wr("holder", "cpuset.cpus.partition", "root");
	}
	return NULL;
}

int main(void)
{
	char pid[32];
	cpu_set_t cpu0_only;
	pthread_t flapper;

	wr(CG "/cgroup.subtree_control", "+cpuset");
	mkdir(CG "/holder", 0755);

	victim = fork();
	if (!victim) {
		pause();
		_exit(0);
	}

	cg_wr("holder", "cpuset.cpus", "0");
	cg_wr("holder", "cpuset.cpus.exclusive", "0");
	cg_wr("holder", "cpuset.cpus.partition", "root");

	snprintf(pid, sizeof(pid), "%d", getpid());
	cg_wr("holder", "cgroup.procs", pid);

	pthread_create(&flapper, NULL, cpu1_flap, NULL);

	CPU_ZERO(&cpu0_only);
	CPU_SET(0, &cpu0_only);
	for (;;)
		sched_setaffinity(victim, sizeof(cpu0_only), &cpu0_only);
	return 0;
}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
  2026-09-01 20:55     ` Farhad Alemi
@ 2026-09-03  3:29       ` Waiman Long
  2026-09-03  8:40         ` Farhad Alemi
  0 siblings, 1 reply; 6+ messages in thread
From: Waiman Long @ 2026-09-03  3:29 UTC (permalink / raw)
  To: Farhad Alemi
  Cc: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	cgroups, linux-kernel

On 9/1/26 4:55 PM, Farhad Alemi wrote:
> Apologies for the delay in response. Upon our run, the patched kernel
> 388b607d107c07aaade04c7f22f344cab6bdccd3 no longer panics when facing
> the attached reproducer.

Thanks for verifying the patch. Would you mind if I added the following 
Tested-by tag to the patch?

Tested-by: Farhad Alemi <farhad.alemi@berkeley.edu>

Thanks,
Longman



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus()
  2026-09-03  3:29       ` Waiman Long
@ 2026-09-03  8:40         ` Farhad Alemi
  0 siblings, 0 replies; 6+ messages in thread
From: Farhad Alemi @ 2026-09-03  8:40 UTC (permalink / raw)
  To: Waiman Long
  Cc: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	cgroups, linux-kernel

I do not mind at all! As long as the Tested-by tag doesn’t imply
exhaustive testing for soundness and completeness, please feel free to
add it. Thanks for asking!

On Wed, Sep 2, 2026 at 8:29 PM Waiman Long <longman@redhat.com> wrote:
>
> On 9/1/26 4:55 PM, Farhad Alemi wrote:
> > Apologies for the delay in response. Upon our run, the patched kernel
> > 388b607d107c07aaade04c7f22f344cab6bdccd3 no longer panics when facing
> > the attached reproducer.
>
> Thanks for verifying the patch. Would you mind if I added the following
> Tested-by tag to the patch?
>
> Tested-by: Farhad Alemi <farhad.alemi@berkeley.edu>
>
> Thanks,
> Longman
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-03  8:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-28 18:03 [PATCH] cgroup/cpuset: Handle cpu hotplug race in guarantee_active_cpus() Waiman Long
2026-08-31  1:50 ` Ridong Chen
2026-09-01  0:08   ` Waiman Long
2026-09-01 20:55     ` Farhad Alemi
2026-09-03  3:29       ` Waiman Long
2026-09-03  8:40         ` Farhad Alemi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®