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

[Jann Horn]

On Fri, Jul 31, 2026 at 4:50 PM Chen Linxuan via B4 Relay
<devnull+me.black-desk.cn@xxxxxxxxxx> 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 comment
> 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
“goto”, and that the “goto” statement can jump between scopes, the
expectation is that usage of “goto” 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?