Re: [PATCH] pidfd: hold exec_update_lock around namespace ioctl

On Fri, Jul 31, 2026 at 4:50=E2=80=AFPM Chen Linxuan via B4 Relay
<[email protected]> wrote:
> The PIDFD_GET_*_NAMESPACE ioctls in pidfd_ioctl() perform a filesystem
> credentials ptrace access check before handing out a namespace file
> descriptor.  The accompanying comment states that the code "mirrors nsfs
> behavior", but, unlike the corresponding procfs paths, it does so without
> holding the target task's exec_update_lock.
>
> proc_ns_get_link() and proc_ns_readlink() both take exec_update_lock for
> reading around the ptrace check and the namespace lookup, so that the
> credentials used for the access decision match those of the task when its
> namespace is read.  Without it, a caller can pass the check against the
> target's old credentials and then read the namespace after the target has
> execve()'d a setuid binary and committed new credentials -- accessing
> namespace information it should have been denied.
>
> Hold exec_update_lock for reading around the ptrace check and the
> namespace lookup so that pidfd truly mirrors nsfs behavior, as the commen=
t
> already claims.  open_namespace() itself runs outside the lock: once a
> namespace reference is obtained it carries its own refcount and is opened
> with the caller's own credentials, so a concurrent execve() on the target
> can no longer affect the outcome.

I think this makes sense.

Given that the rest of this function is written with scope-based
cleanup (https://docs.kernel.org/core-api/cleanup.html), I wonder if
this patch would look cleaner if it also used scope-based cleanup...
that documentation also says:

"Lastly, given that the benefit of cleanup helpers is removal of
=E2=80=9Cgoto=E2=80=9D, and that the =E2=80=9Cgoto=E2=80=9D statement can j=
ump between scopes, the
expectation is that usage of =E2=80=9Cgoto=E2=80=9D and cleanup helpers is =
never mixed
in the same function."

But we probably shouldn't be holding the exec_update_lock across the
open_namespace() call... so I guess this will require either
refactoring this function into two, or indenting most of the function
body, or some explicit "drop this guard" operation.

@Christian, do you have an opinion on this?