The target task can execute a setuid binary between ptrace_may_access()
and get_task_mm(). Protect this critical section with exec_update_lock.

I don't think cpuset_mems_allowed(task) should be called under
exec_update_lock, but this patch just tries to add the minimal fix.

Perhaps we can later add a common helper which can be used by
find_mm_struct() and kernel_migrate_pages().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 mm/migrate.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 8a64291ab5b4..e4cf664fb161 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2557,24 +2557,29 @@ static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes)
 	}
 
 	task = find_get_task_by_vpid(pid);
-	if (!task) {
+	if (!task)
 		return ERR_PTR(-ESRCH);
-	}
 
+	if (down_read_killable(&task->signal->exec_update_lock)) {
+		mm = ERR_PTR(-EINTR);
+		goto out;
+	}
 	/*
 	 * Check if this process has the right to modify the specified
 	 * process. Use the regular "ptrace_may_access()" checks.
 	 */
 	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
 		mm = ERR_PTR(-EPERM);
-		goto out;
+		goto unlock;
 	}
 
 	mm = ERR_PTR(security_task_movememory(task));
 	if (IS_ERR(mm))
-		goto out;
+		goto unlock;
 	*mem_nodes = cpuset_mems_allowed(task);
 	mm = get_task_mm(task);
+unlock:
+	up_read(&task->signal->exec_update_lock);
 out:
 	put_task_struct(task);
 	if (!mm)
-- 
2.52.0

On Tue, May 26, 2026 at 04:42:11PM +0200, Oleg Nesterov wrote:
> The target task can execute a setuid binary between ptrace_may_access()
> and get_task_mm(). Protect this critical section with exec_update_lock.
> 
> I don't think cpuset_mems_allowed(task) should be called under
> exec_update_lock, but this patch just tries to add the minimal fix.
>

Claude suggests this is at least safe and the correct ordering, for
which there already exists code that does the same thing:

   This ordering already exists in the exec path itself: exec_mmap()
   write-locks exec_update_lock, and subsequent memory allocations
   during exec can reach callback_lock through the page allocator's
   cpuset checks (cpuset_zone_allowed etc.).

This is inline with my experience hacking on page_alloc.c and cgroups,
the callback lock is always the inner-most lock - so lgtm.

> Perhaps we can later add a common helper which can be used by
> find_mm_struct() and kernel_migrate_pages().
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Reviewed-by: Gregory Price <gourry@gourry.net>

~Gregory

On Tue, 26 May 2026 16:42:11 +0200 Oleg Nesterov <oleg@redhat.com> wrote:

> The target task can execute a setuid binary between ptrace_may_access()
> and get_task_mm(). Protect this critical section with exec_update_lock.
> 
> I don't think cpuset_mems_allowed(task) should be called under
> exec_update_lock, but this patch just tries to add the minimal fix.
> 
> Perhaps we can later add a common helper which can be used by
> find_mm_struct() and kernel_migrate_pages().
> 

Do you think we should backport this into earlier kernels?

On 05/26, Andrew Morton wrote:
>
> On Tue, 26 May 2026 16:42:11 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > The target task can execute a setuid binary between ptrace_may_access()
> > and get_task_mm(). Protect this critical section with exec_update_lock.
> >
> > I don't think cpuset_mems_allowed(task) should be called under
> > exec_update_lock, but this patch just tries to add the minimal fix.
> >
> > Perhaps we can later add a common helper which can be used by
> > find_mm_struct() and kernel_migrate_pages().
> >
>
> Do you think we should backport this into earlier kernels?

Probably not... The race is very unlikely and iiuc the impact is not
serious...

Up to maintainers.

Oleg.

On Tue, 26 May 2026 16:42:11 +0200 Oleg Nesterov <oleg@redhat.com> wrote:

> The target task can execute a setuid binary between ptrace_may_access()
> and get_task_mm(). Protect this critical section with exec_update_lock.
> 
> I don't think cpuset_mems_allowed(task) should be called under
> exec_update_lock, but this patch just tries to add the minimal fix.
> 
> Perhaps we can later add a common helper which can be used by
> find_mm_struct() and kernel_migrate_pages().
> 

Thanks.  Sashiko thinks we should fix kernel_migrate_pages() also:
	https://sashiko.dev/#/patchset/ahWxQ3JxdR5ff2qf@redhat.com

On 05/26, Andrew Morton wrote:
>
> On Tue, 26 May 2026 16:42:11 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > The target task can execute a setuid binary between ptrace_may_access()
> > and get_task_mm(). Protect this critical section with exec_update_lock.
> >
> > I don't think cpuset_mems_allowed(task) should be called under
> > exec_update_lock, but this patch just tries to add the minimal fix.
> >
> > Perhaps we can later add a common helper which can be used by
> > find_mm_struct() and kernel_migrate_pages().
> >
>
> Thanks.  Sashiko thinks we should fix kernel_migrate_pages() also:
> 	https://sashiko.dev/#/patchset/ahWxQ3JxdR5ff2qf@redhat.com

Of course ;) That is why I have already sent

	[PATCH 0/4] mm/mempolicy: kernel_migrate_pages: fix race between security checks and suid exec
	https://lore.kernel.org/all/ahMt6xyUNnacZU8-@redhat.com/

and mentioned this in 0/4.

Sashiko has concern about 3/4 in the series above.

I personally do not think this is a problem... Without hidepid != 0
/proc/pid/status reports the same "info leak".

But may be I am wrong. Still waiting for review from maintainers.

Oleg.