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);
+
+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
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; > +} > + [...]
> On Jul 4, 2025, at 10:40 AM, Yonghong Song <yonghong.song@linux.dev> wrote: [...] >> +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. I am not quite following the question. In the code below: 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; /* We either continue, here */ } /* 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 /* Or return here */ } So we will not hit prepend_name(). Does this answer the question? > > 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(). Yeah, I will try to add more tests in the next revision. Thanks, Song
On 7/6/25 4:54 PM, Song Liu wrote: > >> On Jul 4, 2025, at 10:40 AM, Yonghong Song <yonghong.song@linux.dev> wrote: > [...] >>> +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. > I am not quite following the question. In the code below: > > 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; > /* We either continue, here */ > > } > /* 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 > /* Or return here */ > } > > So we will not hit prepend_name(). Does this answer the > question? > >> 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(). Looks like __path_walk_parent() works for the root of mounted fs. If this is the case, the implementation is correct. It could be good to add some comments to clarify. > Yeah, I will try to add more tests in the next revision. > > Thanks, > Song >
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@brown.name> > Signed-off-by: Song Liu <song@kernel.org> Looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.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@suse.com> SUSE Labs, CR
Hi Jan, On Tue, Jun 24, 2025 at 5:18 AM Jan Kara <jack@suse.cz> wrote: > > 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@brown.name> > > Signed-off-by: Song Liu <song@kernel.org> > > Looks good to me. Feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> Thanks for the review! [...] > > + * > > + * 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. We have RESOLVE_NO_XDEV in uapi/linux/openat2.h, so I guess we cannot really remove it? Thanks, Song [...]
On Tue 24-06-25 10:37:36, Song Liu wrote: > On Tue, Jun 24, 2025 at 5:18 AM Jan Kara <jack@suse.cz> wrote: > > > + * > > > + * 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. > > We have RESOLVE_NO_XDEV in uapi/linux/openat2.h, so I guess we > cannot really remove it? I didn't mean to remove the LOOKUP_NO_XDEV flag, I meant to not update the passed path if LOOKUP_NO_XDEV is set, we are crossing the mountpoint and thus returning -EXDEV. As far as I've checked once we return error, everybody just path_put()s the nd->path so its update is just pointless. But there are many (indirect) callers so I might have missed some case. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
Hi Song, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Song-Liu/namei-Introduce-new-helper-function-path_walk_parent/20250617-141322 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250617061116.3681325-2-song%40kernel.org patch subject: [PATCH v5 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() config: loongarch-randconfig-r072-20250618 (https://download.01.org/0day-ci/archive/20250618/202506180814.GoByWn1r-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 12.4.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250618/202506180814.GoByWn1r-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202506180814.GoByWn1r-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: fs/namei.c:2072 function parameter 'path' not described in '__path_walk_parent' >> Warning: fs/namei.c:2072 function parameter 'root' not described in '__path_walk_parent' >> Warning: fs/namei.c:2072 function parameter 'flags' not described in '__path_walk_parent' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.