[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <htn4tupeslsrhyzrqt7pi34tye7tpp7amziiwflfpluj3u2nhs@e2axcpfuucv5>
Date: Tue, 24 Jun 2025 14:18:17 +0200
From: Jan Kara <jack@...e.cz>
To: Song Liu <song@...nel.org>
Cc: bpf@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, kernel-team@...a.com,
andrii@...nel.org, eddyz87@...il.com, ast@...nel.org, daniel@...earbox.net,
martin.lau@...ux.dev, viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
kpsingh@...nel.org, mattbobrowski@...gle.com, m@...wtm.org, neil@...wn.name
Subject: Re: [PATCH v5 bpf-next 1/5] namei: Introduce new helper function
path_walk_parent()
On Mon 16-06-25 23:11:12, 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@...wn.name>
> Signed-off-by: Song Liu <song@...nel.org>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
One note below:
> -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.
> + */
The behavior with LOOKUP_NO_XDEV is kind of odd (not your fault) and
interestingly I wasn't able to find a place that would depend on the path
being updated in that case. So either I'm missing some subtle detail (quite
possible) or we can clean that up in the future.
Honza
> +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);
> +
> +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;
> +}
> +
> +static struct dentry *follow_dotdot(struct nameidata *nd)
> +{
> + struct dentry *parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
> +
> + if (IS_ERR(parent))
> + return parent;
> if (unlikely(!path_connected(nd->path.mnt, parent))) {
> dput(parent);
> return ERR_PTR(-ENOENT);
> }
> + nd->inode = nd->path.dentry->d_inode;
> return parent;
> -
> -in_root:
> - if (unlikely(nd->flags & LOOKUP_BENEATH))
> - return ERR_PTR(-EXDEV);
> - return dget(nd->path.dentry);
> }
>
> static const char *handle_dots(struct nameidata *nd, int type)
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 5d085428e471..ca68fa4089e0 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -85,6 +85,8 @@ extern int follow_down_one(struct path *);
> extern int follow_down(struct path *path, unsigned int flags);
> extern int follow_up(struct path *);
>
> +int path_walk_parent(struct path *path, const struct path *root);
> +
> extern struct dentry *lock_rename(struct dentry *, struct dentry *);
> extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
> extern void unlock_rename(struct dentry *, struct dentry *);
> --
> 2.47.1
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists