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            | 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))) {
 		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 *);
-- 
2.47.1

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@brown.name>
> Signed-off-by: Song Liu <song@kernel.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?

(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 *);

On Sat, Jun 14, 2025 at 11:36 AM Tingmao Wang <m@maowtm.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@brown.name>
> > Signed-off-by: Song Liu <song@kernel.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 *);
>

On Thu, 12 Jun 2025, 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            | 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))) {
>  		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;
> +}

I think the public function should return 0 on success and -error on
failure.  That is a well established pattern.  I also think you
shouldn't assume that all callers will want the same flags.

And it isn't clear to me why you want to path_put() on failure.

I wonder if there might be other potential users in the kernel.
If so we should consider how well the interface meets their needs.

autofs, devpts, nfsd, landlock all call follow_up...
maybe they should be using the new interface...
nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

Just a thought..

NeilBrown



> +
> +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 *);
> -- 
> 2.47.1
> 
> 

On Thu, Jun 12, 2025 at 5:11 PM NeilBrown <neil@brown.name> wrote:
[...]
> > +
> > +false_out:
> > +     path_put(path);
> > +     memset(path, 0, sizeof(*path));
> > +     return false;
> > +}
>
> I think the public function should return 0 on success and -error on
> failure.  That is a well established pattern.

Yeah, I think we can use this pattern.

> I also think you
> shouldn't assume that all callers will want the same flags.

__path_walk_parent() only handles two LOOKUP_ flags, so
it is a bit weird to allow all the flags. But if folks think this is a
good idea, I don't have strong objections to taking various flags.

>
> And it isn't clear to me why you want to path_put() on failure.

In earlier versions, we would keep "path" unchanged when the
walk stopped. However, this is not the case in this version
(choose_mountpoint() => in_root => return -EXDEV). So I
decided to just release it, so that we will not leak a path that
the walk should not get to.

>
> I wonder if there might be other potential users in the kernel.
> If so we should consider how well the interface meets their needs.
>
> autofs, devpts, nfsd, landlock all call follow_up...
> maybe they should be using the new interface...
> nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

AFAICT, autofs and devpts can just use follow_up().
For nfsd, nfsd_lookup_parent() and nfsd4_encode_pathname4() can
use path_walk_parent. And 2/5 covers landlock.

I think we can update nfsd in a follow up patch, just to keep this set
simpler.

Thanks,
Song

> Just a thought..

[...]