| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPhsuW4a9kUyNhw1kZuyrFLsWF4-FseT0Cb2PK1TpZ6VZmT1AQ@mail.gmail.com>
Date: Mon, 16 Jun 2025 22:08:27 -0700
From: Song Liu <song@...nel.org>
To: Tingmao Wang <m@...wtm.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, amir73il@...il.com, repnop@...gle.com,
jlayton@...nel.org, josef@...icpanda.com, mic@...ikod.net, gnoack@...gle.com,
neil@...wn.name
Subject: Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
On Sat, Jun 14, 2025 at 11:36 AM Tingmao Wang <m@...wtm.org> wrote:
>
> On 6/11/25 23:02, 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>
> > ---
> > fs/namei.c | 99 +++++++++++++++++++++++++++++++++++++------
> > include/linux/namei.h | 2 +
> > 2 files changed, 87 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 4bb889fc980b..bc65361c5d13 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -2048,36 +2048,107 @@ 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);
> > - if (unlikely(!path_connected(nd->path.mnt, parent))) {
> > + parent = dget_parent(path->dentry);
> > + if (unlikely(!path_connected(path->mnt, parent))) {
>
> This is checking path_connected here but also in follow_dotdot,
> path_connected is checked again. Is this check meant to be here? It will
> also change the landlock behaviour right?
Good catch! Removed the check in v5.
>
> (For some reason patch 2 rejects when I tried to apply it on v6.16-rc1, so
> I haven't actually tested this patch to see if this is really an issue)
>
> > dput(parent);
> > return ERR_PTR(-ENOENT);
> > }
> > return parent;
> >
> > in_root:
> > - if (unlikely(nd->flags & LOOKUP_BENEATH))
> > + if (unlikely(flags & LOOKUP_BENEATH))
> > return ERR_PTR(-EXDEV);
> > - return dget(nd->path.dentry);
> > + 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
> > + * released and zero'ed.
> > + *
> > + * Returns:
> > + * true - if @path is updated to its parent.
> > + * false - if @path is already the root (real root or @root).
> > + */
> > +bool 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))
> > + goto false_out;
> > +
> > + if (parent == path->dentry) {
> > + dput(parent);
> > + goto false_out;
> > + }
> > + dput(path->dentry);
> > + path->dentry = parent;
> > + return true;
> > +
> > +false_out:
> > + path_put(path);
> > + memset(path, 0, sizeof(*path));
> > + return false;
> > +}
> > +
> > +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;
> > }
> >
> > static const char *handle_dots(struct nameidata *nd, int type)
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index 5d085428e471..cba5373ecf86 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 *);
> >
> > +bool 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 *);
>
Powered by blists - more mailing lists