All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: oleg@redhat.com, mingo@redhat.com, peterz@infradead.org,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	torvalds@linux-foundation.org, qsa@qualys.com, kees@kernel.org
Cc: linux-kernel@vger.kernel.org, imv4bel@gmail.com
Subject: Re: [PATCH] ptrace: prefer live sibling mm over user_dumpable when task->mm is NULL
Date: Fri, 15 May 2026 11:18:39 +0900	[thread overview]
Message-ID: <agaCf9239m6_oA_C@v4bel> (raw)
In-Reply-To: <agZjJdaIGCvf0z_1@v4bel>

On Fri, May 15, 2026 at 09:04:53AM +0900, Hyunwoo Kim wrote:
> task_still_dumpable() reads task->user_dumpable when task->mm is NULL.
> That cache is written exactly once in exit_mm(): a single
> get_dumpable(mm) load taken just before task->mm is cleared.
> 
> The load is not ordered against set_dumpable() writes performed by
> CLONE_VM siblings, either via commit_creds() (any uid/gid/fsuid/fsgid
> or capability transition) or via prctl(PR_SET_DUMPABLE). If a sibling
> stores SUID_DUMP_DISABLE after the exiting thread observed
> SUID_DUMP_USER, the live shared mm and the cached value diverge with
> no later refresh, so task_still_dumpable() keeps returning the stale
> SUID_DUMP_USER answer for the exiting task.
> 
> In the task->mm == NULL branch, walk the thread group under
> rcu_read_lock and spin_trylock(&t->alloc_lock); use the first sibling
> that still holds the shared mm and read its current get_dumpable().
> Pin the mm's user_ns with get_user_ns() so it can outlive the
> locked region (the actual ptrace_has_cap() may sleep). Keep
> task->user_dumpable only as the fallback when no live sibling mm is
> observable: a single-threaded exit, a PF_KTHREAD, or all siblings
> simultaneously contended on alloc_lock.
> 
> Fixes: 31e62c2ebbfd ("ptrace: slightly saner 'get_dumpable()' logic")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  kernel/ptrace.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 130043bfc209..2b2d8402b9ee 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -272,15 +272,59 @@ static bool ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
>  	return ns_capable(ns, CAP_SYS_PTRACE);
>  }
>  
> +static bool task_sibling_mm_dumpable(struct task_struct *task,
> +				     unsigned int mode, bool *result)
> +{
> +	struct task_struct *t;
> +	bool found = false;
> +
> +	rcu_read_lock();
> +	for_each_thread(task, t) {
> +		struct mm_struct *sib_mm;
> +		struct user_namespace *sib_uns;
> +		int sib_dumpable;
> +
> +		if (t == task)
> +			continue;
> +		if (!spin_trylock(&t->alloc_lock))
> +			continue;
> +		sib_mm = t->mm;
> +		if (!sib_mm) {
> +			spin_unlock(&t->alloc_lock);
> +			continue;
> +		}
> +		sib_dumpable = get_dumpable(sib_mm);
> +		sib_uns = get_user_ns(sib_mm->user_ns);
> +		spin_unlock(&t->alloc_lock);
> +
> +		if (sib_dumpable == SUID_DUMP_USER)
> +			*result = true;
> +		else
> +			*result = ptrace_has_cap(sib_uns, mode);
> +		put_user_ns(sib_uns);
> +		found = true;
> +		break;
> +	}
> +	rcu_read_unlock();
> +	return found;
> +}
> +
>  static bool task_still_dumpable(struct task_struct *task, unsigned int mode)
>  {
>  	struct mm_struct *mm = task->mm;
> +	bool sib_result;
> +
>  	if (mm) {
>  		if (get_dumpable(mm) == SUID_DUMP_USER)
>  			return true;
>  		return ptrace_has_cap(mm->user_ns, mode);
>  	}
>  
> +	/* user_dumpable can be stale; prefer a live sibling mm if any. */
> +	if (!(task->flags & PF_KTHREAD) &&
> +	    task_sibling_mm_dumpable(task, mode, &sib_result))
> +		return sib_result;
> +
>  	if (task->user_dumpable)
>  		return true;
>  	return ptrace_has_cap(&init_user_ns, mode);
> -- 
> 2.43.0
> 

To avoid confusion: a v2 patch has been submitted.

https://lore.kernel.org/all/agZ_Ug3EzCYn-Jkg@v4bel/


Best regards,
Hyunwoo Kim

      reply	other threads:[~2026-05-15  2:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  0:04 [PATCH] ptrace: prefer live sibling mm over user_dumpable when task->mm is NULL Hyunwoo Kim
2026-05-15  2:18 ` Hyunwoo Kim [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=agaCf9239m6_oA_C@v4bel \
    --to=imv4bel@gmail.com \
    --cc=juri.lelli@redhat.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=qsa@qualys.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.guittot@linaro.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.