Re: [PATCH v4 3/3] pidfd: Add task path ioctls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pidfd: missing d_weak_revalidate() check in pidfd_ioctl() task path opens
--

commit ce70068de308fcb3a1353124233724116074eb7b
Author: Chen Linxuan <me@black-desk.cn>

pidfd: Add task path ioctls

This commit adds the PIDFD_GET_EXE, PIDFD_GET_CWD, and PIDFD_GET_ROOT ioctls
to return close-on-exec O_PATH file descriptors referencing a target task's
executable, current working directory, and root directory.

> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index b70ec50f64da9..1e5b0dcb99756 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c

[ ... ]

> @@ -707,6 +733,18 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	if (_IOC_NR(cmd) == _IOC_NR(PIDFD_GET_INFO))
>  		return pidfd_info(file, cmd, arg);
>  
> +	switch (cmd) {
> +	case PIDFD_GET_EXE:
> +	case PIDFD_GET_CWD:
> +	case PIDFD_GET_ROOT:
> +		error = pidfd_get_task_path(pidfd_pid(file), cmd, arg, &path);
> +		if (error)
> +			return error;
> +
> +		return FD_ADD(O_CLOEXEC,
> +			      dentry_open(&path, O_PATH, current_cred()));
> +	}

[Severity: Medium]
Should pidfd_ioctl() check DCACHE_OP_WEAK_REVALIDATE and invoke
d_weak_revalidate() on path.dentry before calling dentry_open()?

When a task's root, cwd, or exe is opened through /proc/<pid>/{root,cwd,exe},
proc_pid_get_link() jumps to the target path via nd_jump_link(), setting
ND_JUMPED in struct nameidata. Before do_o_path() calls vfs_open(),
path_lookupat() calls complete_walk() in fs/namei.c:

	if (likely(!(nd->state & ND_JUMPED)))
		return 0;

	if (likely(!(dentry->d_flags & DCACHE_OP_WEAK_REVALIDATE)))
		return 0;

	status = dentry->d_op->d_weak_revalidate(dentry, nd->flags);

Because pidfd_ioctl() passes &path directly to
dentry_open() -> vfs_open() -> do_dentry_open() with O_PATH,
d_weak_revalidate() is never called.

In particular, failfs relies on failfs_d_weak_revalidate() in fs/failfs.c to
prevent a task's root or cwd in failfs from being opened with O_PATH:

static int failfs_d_weak_revalidate(struct dentry *dentry, unsigned int flags)
{
	/*
	 * The root is only ever reached as a path-walk terminal by jumping
	 * to it: as "/" when it is the caller's root, or through a
	 * /proc/<pid>/{root,cwd} magic link. ->permission() already fails
	 * every walk of a component, but a jump lands on the root without
	 * one. Refuse here too so the root cannot be pinned by an O_PATH
	 * open or encoded into a file handle.
	 */
	return -EOPNOTSUPP;
}

If a task sets its working directory or root to failfs via
fchdir(FD_FAILFS_ROOT) or fchroot(FD_FAILFS_ROOT, 0), calling
ioctl(pidfd, PIDFD_GET_CWD, 0) or ioctl(pidfd, PIDFD_GET_ROOT, 0) bypasses
failfs_d_weak_revalidate() and returns an O_PATH file descriptor pinning
failfs_root_path, in addition to skipping weak revalidation on filesystems
such as NFS, 9p, and overlayfs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-pidfd-get-paths-v4-0-e2a6a47e3c95@black-desk.cn?part=3