Re: [PATCH] futex: don't leak robust_list pointer on exec race

Pranav Tyagi
2025-07-30T14:51:35Z

On Fri, Jun 13, 2025 at 6:38 PM Thomas Gleixner <tglx@linutronix.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@linutronix.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.
> >>
> >> 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.
>
Hi,

I face a small issue while refactoring the common code in a helper.

The main obstacle to a full refactor is that the native and compat
syscalls use different user-visible types (size_t vs compat_size_t,
struct robust_list_head * vs compat_uptr_t). Because put_user() is
type-checked at compile-time, I can’t unify both into one function
without either unsafe casting or weakening type safety (this is as far
as I understand).

The best I can do is refactor the common task lookup/permission
logic into a helper, and leave ABI-specific put_user() calls in thin wrappers.

I’d be grateful for your suggestion.

Regards
Pranav Tyagi

> Thanks,
>
> tglx