mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: "Thomas Gleixner" <tglx@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Frederic Weisbecker" <frederic@kernel.org>,
	"Tejun Heo" <tj@kernel.org>, "Michal Koutný" <mkoutny@suse.com>,
	"Ridong Chen" <ridong.chen@linux.dev>,
	"Guopeng Zhang" <guopeng.zhang@linux.dev>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
Date: Sun, 8 Nov 2026 23:27:21 -0500	[thread overview]
Message-ID: <06324bf7-ba8e-4485-b916-8a03eed755ad@redhat.com> (raw)
In-Reply-To: <20261109023703.1551928-1-mathieu.desnoyers@efficios.com>

On 11/8/26 9:37 PM, Mathieu Desnoyers wrote:
> [ Resent with the lists in Cc. The first copy went to Waiman only. ]
>
> On 2026-11-08 13:08, Waiman Long wrote:
> [...]
>
> This is not about the patch itself, but about the mask it reads. The
> Sashiko AI reviews of this patch and of my sched/isolation patch [1]
> both flag a race on the HK_TYPE_DOMAIN cpumask. I checked it against
> the code. It is real in some configurations, and older than this patch.
>
> Since commit a84097e625f2 ("cgroup/cpuset: Call housekeeping_update()
> without holding cpus_read_lock"), housekeeping_update() replaces the
> mask and frees the old one with only cpuset_top_mutex held. Its message
> says that CPU hotplug can run in parallel. But
> housekeeping_dereference_check() still accepts cpus_write_lock, or
> cpuset_mutex, as enough to read the mask without RCU. _cpu_down() reads
> it that way, with or without this patch:
>
>    CPU 0                                  CPU 1
>
>    write to cpuset.cpus.partition
>      cpuset_update_sd_hk_unlock()
>        mutex_unlock(&cpuset_mutex)
>        cpus_read_unlock()
>                                           echo 0 > .../cpuN/online
>                                             _cpu_down()
>                                               cpus_write_lock()
>                                               m = housekeeping_cpumask(
>                                                     HK_TYPE_DOMAIN)
>                                               <preempted>
>        housekeeping_update()
>          rcu_assign_pointer(...)
>          synchronize_rcu()
>            CPU 1 is not an RCU reader
>          flushes workqueues
>          tmigr_isolated_exclude_cpumask()
>            the !CONFIG_NO_HZ_COMMON stub
>          kfree(old)
>                                               cpumask_any_and(..., m)
>                                                 reads freed memory
>
> With CONFIG_NO_HZ_COMMON, tmigr_isolated_exclude_cpumask() takes
> cpus_read_lock() before kfree(old). That waits for CPU 1. So most
> configurations are safe, but by accident. Without CONFIG_NO_HZ_COMMON,
> nothing makes kfree(old) wait. The same holds when one of its two
> allocations fails: it returns -ENOMEM before it takes the lock.
>
> This comes from reading the code. I did not try to reproduce it.
>
> A fix could make housekeeping_update() take and release
> cpus_read_lock() before kfree(old). That waits for any hotplug reader
> of the old mask, whatever the timer code does. It holds only
> cpuset_top_mutex there, which comes first in the cpuset lock order. I
> did not find a reader that holds cpuset_mutex without the hotplug
> lock, but I did not check every caller. I can send a patch, if you
> agree with that direction.
>
> [1] https://lore.kernel.org/all/20261108185129.1549419-1-mathieu.desnoyers@efficios.com/

Yes, I am aware of the sashiko report of this UAF problem. Taking and 
releasing cpus_read_lock() before kfree() is a possible workaround. 
Another possible fix is to take out cpuset_mutex and cpus_write_lock 
from housekeeping_dereference_check() and add rcu_read_lock/_unlock 
around the housekeeping cpumask access. The latter approach is what I am 
planning to do.

Cheers,
Longman


      reply	other threads:[~2026-11-09  4:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-08 18:08 Waiman Long
2026-11-08 18:19 ` Waiman Long
2026-11-08 19:18 ` Bradley Morgan
2026-11-09  2:37 ` Mathieu Desnoyers
2026-11-09  4:27   ` Waiman Long [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06324bf7-ba8e-4485-b916-8a03eed755ad@redhat.com \
    --to=longman@redhat.com \
    --cc=cgroups@vger.kernel.org \
    --cc=frederic@kernel.org \
    --cc=guopeng.zhang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mkoutny@suse.com \
    --cc=peterz@infradead.org \
    --cc=ridong.chen@linux.dev \
    --cc=tglx@kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®