Message-ID: <CAH4c4j+P2MJWZWv6M2s+wOdfCF6q6Wyq3=VsB=uNCtEH1LMp-g@mail.gmail.com>
Date: Wed, 18 Jun 2025 11:50:31 +0530
From: Pranav Tyagi <pranav.tyagi03@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, 
	Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>, 
	André Almeida <andrealmeid@...lia.com>, 
	linux-kernel@...r.kernel.org, jann@...jh.net, keescook@...omium.org, 
	skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] futex: don't leak robust_list pointer on exec race

On Fri, Jun 13, 2025 at 6:38 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Wed, Jun 11 2025 at 19:33, Pranav Tyagi wrote:
> > On Mon, Jun 9, 2025 at 3:15 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> > Does the revised version below address the concerns more effectively
> > or does it still need a bit more seasoning?
> >
> > "Currently, sys_get_robust_list() and compat_get_robust_list() perform a
> > ptrace_may_access() check to verify if the calling task is allowed to
> > query another task’s robust_list pointer. However, this check is racy
> > against a concurrent exec() in the target process.
> >
> > During exec(), a task's credentials and memory mappings can change, and
> > the task may transition from a non-privileged binary to a privileged one
> > (e.g., a setuid binary). If get_robust_list() performs ptrace_may_access()
> > before this transition, it may wrongly allow access to sensitive
> > information after the target becomes privileged.
> >
> > To address this, a read lock is taken on signal->exec_update_lock prior
> > to invoking ptrace_may_access() and accessing the robust_list. This
> > ensures that the target task's exec state remains stable during the
> > check, allowing for consistent and synchronized validation of
> > credentials."
>
> That's way better, but it still does not explain what the consequences
> of the racy access are.

"The racy access allows a malicious observer to exploit a window during
which ptrace_may_access() passes before a target process transitions to
a privileged state via exec(). For example, consider a non-privileged
task T that is about to execute a setuid-root binary. An attacker task A
calls get_robust_list(T) while T is still unprivileged. Since
ptrace_may_access() checks permissions based on current credentials, it
succeeds.

However, if T begins its exec() immediately afterward, it becomes
privileged and may change its memory mappings. Because get_robust_list()
proceeds to access T->robust_list without synchronizing with exec(), it
may read user-space pointers from a now-privileged process. This
violates the intended post-exec access restrictions and could expose
sensitive memory addresses or be used as a primitive in a larger exploit
chain.

Thus, the race can lead to unauthorized disclosure of information across
privilege boundaries and poses a potential security risk. Taking a read
lock on exec_update_lock ensures that the target task's exec state
remains stable during both the permission check and the access,
preventing this inconsistent and potentially unsafe access."

I hope this explains the consequences of the race condition. If it is fine,
I'll add it to changelog when I resubmit with the helper function.

> >>
> >> You really did not have a better idea than copying all of that logic
> >> into the compat code?
> >
> > As I’m still learning, I wasn’t quite sure how to avoid
> > duplication there. Would factoring out the common logic into a helper function
> > be the right direction? I’d be grateful for your suggestion.
>
> Exactly.
>
> Thanks,
>
>         tglx

Powered by blists - more mailing lists