Re: [PATCH v5 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()

[Yonghong Song <yonghong.song@linux.dev>]

On 6/16/25 11:11 PM, Song Liu wrote:
> This helper walks an input path to its parent. Logic are added to handle
> walking across mount tree.
>
> This will be used by landlock, and BPF LSM.
>
> Suggested-by: Neil Brown <neil@brown.name>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>   fs/namei.c            | 95 +++++++++++++++++++++++++++++++++++--------
>   include/linux/namei.h |  2 +
>   2 files changed, 79 insertions(+), 18 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 4bb889fc980b..d0557c0b5cc8 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2048,36 +2048,95 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
>   	return nd->path.dentry;
>   }
>   
> -static struct dentry *follow_dotdot(struct nameidata *nd)
> +/**
> + * __path_walk_parent - Find the parent of the given struct path
> + * @path  - The struct path to start from
> + * @root  - A struct path which serves as a boundary not to be crosses.
> + *        - If @root is zero'ed, walk all the way to global root.
> + * @flags - Some LOOKUP_ flags.
> + *
> + * Find and return the dentry for the parent of the given path
> + * (mount/dentry). If the given path is the root of a mounted tree, it
> + * is first updated to the mount point on which that tree is mounted.
> + *
> + * If %LOOKUP_NO_XDEV is given, then *after* the path is updated to a new
> + * mount, the error EXDEV is returned.
> + *
> + * If no parent can be found, either because the tree is not mounted or
> + * because the @path matches the @root, then @path->dentry is returned
> + * unless @flags contains %LOOKUP_BENEATH, in which case -EXDEV is returned.
> + *
> + * Returns: either an ERR_PTR() or the chosen parent which will have had
> + * the refcount incremented.
> + */
> +static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
>   {
> -	struct dentry *parent;
> -
> -	if (path_equal(&nd->path, &nd->root))
> +	if (path_equal(path, root))
>   		goto in_root;
> -	if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
> -		struct path path;
> +	if (unlikely(path->dentry == path->mnt->mnt_root)) {
> +		struct path new_path;
>   
> -		if (!choose_mountpoint(real_mount(nd->path.mnt),
> -				       &nd->root, &path))
> +		if (!choose_mountpoint(real_mount(path->mnt),
> +				       root, &new_path))
>   			goto in_root;
> -		path_put(&nd->path);
> -		nd->path = path;
> -		nd->inode = path.dentry->d_inode;
> -		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
> +		path_put(path);
> +		*path = new_path;
> +		if (unlikely(flags & LOOKUP_NO_XDEV))
>   			return ERR_PTR(-EXDEV);
>   	}
>   	/* rare case of legitimate dget_parent()... */
> -	parent = dget_parent(nd->path.dentry);
> +	return dget_parent(path->dentry);

I have some confusion with this patch when crossing mount boundary.

In d_path.c, we have

static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
                           const struct path *root, struct prepend_buffer *p)
{
         while (dentry != root->dentry || &mnt->mnt != root->mnt) {
                 const struct dentry *parent = READ_ONCE(dentry->d_parent);

                 if (dentry == mnt->mnt.mnt_root) {
                         struct mount *m = READ_ONCE(mnt->mnt_parent);
                         struct mnt_namespace *mnt_ns;

                         if (likely(mnt != m)) {
                                 dentry = READ_ONCE(mnt->mnt_mountpoint);
                                 mnt = m;
                                 continue;
                         }
                         /* Global root */
                         mnt_ns = READ_ONCE(mnt->mnt_ns);
                         /* open-coded is_mounted() to use local mnt_ns */
                         if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
                                 return 1;       // absolute root
                         else
                                 return 2;       // detached or not attached yet
                 }

                 if (unlikely(dentry == parent))
                         /* Escaped? */
                         return 3;

                 prefetch(parent);
                 if (!prepend_name(p, &dentry->d_name))
                         break;
                 dentry = parent;
         }
         return 0;
}

At the mount boundary and not at root mount, the code has
	dentry = READ_ONCE(mnt->mnt_mountpoint);
	mnt = m; /* 'mnt' will be parent mount */
	continue;

After that, we have
	const struct dentry *parent = READ_ONCE(dentry->d_parent);
	if (dentry == mnt->mnt.mnt_root) {
		/* assume this is false */
	}
	...
	prefetch(parent);
         if (!prepend_name(p, &dentry->d_name))
                 break;
         dentry = parent;

So the prepend_name(p, &dentry->d_name) is actually from mnt->mnt_mountpoint.

In your above code, maybe we should return path->dentry in the below if statement?

         if (unlikely(path->dentry == path->mnt->mnt_root)) {
                 struct path new_path;

                 if (!choose_mountpoint(real_mount(path->mnt),
                                        root, &new_path))
                         goto in_root;
                 path_put(path);
                 *path = new_path;
                 if (unlikely(flags & LOOKUP_NO_XDEV))
                         return ERR_PTR(-EXDEV);
+		return path->dentry;
         }
         /* rare case of legitimate dget_parent()... */
         return dget_parent(path->dentry);

Also, could you add some selftests cross mount points? This will
have more coverages with __path_walk_parent().

> +
> +in_root:
> +	if (unlikely(flags & LOOKUP_BENEATH))
> +		return ERR_PTR(-EXDEV);
> +	return dget(path->dentry);
> +}
> +
> +/**
> + * path_walk_parent - Walk to the parent of path
> + * @path: input and output path.
> + * @root: root of the path walk, do not go beyond this root. If @root is
> + *        zero'ed, walk all the way to real root.
> + *
> + * Given a path, find the parent path. Replace @path with the parent path.
> + * If we were already at the real root or a disconnected root, @path is
> + * not changed.
> + *
> + * Returns:
> + *  0  - if @path is updated to its parent.
> + *  <0 - if @path is already the root (real root or @root).
> + */
> +int path_walk_parent(struct path *path, const struct path *root)
> +{
> +	struct dentry *parent;
> +
> +	parent = __path_walk_parent(path, root, LOOKUP_BENEATH);
> +
> +	if (IS_ERR(parent))
> +		return PTR_ERR(parent);
> +
> +	if (parent == path->dentry) {
> +		dput(parent);
> +		return -ENOENT;
> +	}
> +	dput(path->dentry);
> +	path->dentry = parent;
> +	return 0;
> +}
> +

[...]