Re: [PATCH] proc: protect ptrace_may_access() with exec_update_lock (part 1)

On Mon, Jul 6, 2026 at 7:07 PM Magnus Lindholm <[email protected]> wrote:
>
> Hi,
>
> while testing my Alpha generic-entry series on top of v7.2-rc1, I noticed
> that several strace --pidns-translation tests started failing. The same
> generic-entry series on top of v7.1-rc1 passes these tests.
>
> I bisected the regression between v7.1-rc1 and v7.2-rc1, always applying
> the same generic-entry series before testing, and the first bad commit is:
>
>   6650527444dadc63d84aa939d14ecba4fadb2f69
>   proc: protect ptrace_may_access() with exec_update_lock (part 1)
>
> Examples of failing strace tests include:
>
>   signal_receive--pidns-translation.gen.test
>   so_peercred--pidns-translation.gen.test
>   tgkill--pidns-translation.gen.test
>   tkill--pidns-translation.gen.test
>   fcntl--pidns-translation.gen.test
>   xet_robust_list--pidns-translation.gen.test
>   xetpgid--pidns-translation.gen.test
>   xetpriority--pidns-translation.gen.test
>
> One simple reproducer is:
>
>   cd strace/tests
>   ./xetpgid--pidns-translation.gen.test
>
> The failure looks like this:
>
>   ../../src/strace: NS_* ioctl commands are not supported by the kernel
>
> and the decoded output lacks the expected pidns translation comments, e.g.:
>
>   - getpgid(2 /* 6 in strace's PID NS */) = 0
>   + getpgid(2) = 0
>
> Looking at the patch, the relevant part seems to be the change in
> fs/proc/namespaces.c: proc_ns_get_link() and proc_ns_readlink() now take
> task->signal->exec_update_lock around the ptrace_may_access() check and
> namespace link/readlink handling. strace's --decode-pids=pidns code appears
> to rely on accessing /proc/<pid>/ns/pid for short-lived tracees in a nested
> PID namespace, so this looks like a plausible connection to the failure.
>
> The kernel has the relevant namespace options enabled:
>
>   CONFIG_NAMESPACES=y
>   CONFIG_USER_NS=y
>   CONFIG_PID_NS=y
>   CONFIG_CHECKPOINT_RESTORE=y
>   CONFIG_PROC_FS=y
>
> I also tested the basic nsfs ioctls with a small standalone program, both
> outside and inside "unshare -Urpf", and NS_GET_NSTYPE,
> NS_GET_PID_IN_PIDNS and NS_GET_PID_FROM_PIDNS all work there. So the failure
> does not look like missing namespace support or a simple ioctl-number issue;
> it seems specific to the proc/ns access pattern used by strace's pidns
> translation code.
>
> #regzbot introduced: 6650527444dadc63d84aa939d14ecba4fadb2f69

Hmm... running strace on strace, I see this:

669   openat(AT_FDCWD, "/proc/469/ns/pid", O_RDONLY) = 5
669   fstat(5, {st_dev=makedev(0, 0x15), st_ino=1792,
st_mode=S_IFDIR|0511, st_nlink=2, st_uid=1000, st_gid=1000,
st_blksize=1024, st_blocks=
0, st_size=0, st_atime=1783357140 /*
2026-07-06T16:59:00.713036792+0000 */, st_atime_nsec=713036792,
st_mtime=1783357140 /* 2026-07-06T16:59:
00.713036792+0000 */, st_mtime_nsec=713036792, st_ctime=1783357140 /*
2026-07-06T16:59:00.713036792+0000 */, st_ctime_nsec=713036792}) = 0
669   ioctl(5, NS_GET_PARENT)           = -1 ENOTTY (Inappropriate
ioctl for device)
669   write(2, ", child_tidptr=0x7faa62d54a10) = 7", 34) = 34
669   write(2, "strace: NS_* ioctl commands are not supported by the
kernel\n", 60) = 60

So... /proc/469/ns/pid resolves to a directory somehow? That seems very wrong.

root@vm:~# cd /proc/496/ns/pid/
root@vm:/proc/496/ns/pid#

AAaaah, darn, ok, I see. The patch contains this change:

```
 static const char *proc_ns_get_link(struct dentry *dentry,
                                    struct inode *inode,
                                    struct delayed_call *done)
 {
        const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
        struct task_struct *task;
        struct path ns_path;
        int error = -EACCES;

        if (!dentry)
                return ERR_PTR(-ECHILD);

        task = get_proc_task(inode);
        if (!task)
                return ERR_PTR(-EACCES);

+       error = down_read_killable(&task->signal->exec_update_lock);
+       if (error)
+               goto out_put_task;
+
        if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
                goto out;
```

and that is wrong because previously, the ptrace_may_access() check
relied on "error" still being "error = -EACCES" from the
initialization at the top of the function, but now it is 0 from
down_read_killable(), so now a ptrace permission check failure causes
us to return with 0 without actually having called nd_jump_link(),
which I think means we end up staying in /proc/$pid/ns?

Sigh, I will send a fix.