Re: [PATCH 02/10] fs: add path_create(), path_move() and path_clone()

[NeilBrown]

On Fri, 09 Oct 2026, Mateusz Guzik wrote:
> On Tue, Sep 15, 2026 at 2:10 AM NeilBrown <neilb@xxxxxxxxxxx> wrote:
> >
> > On Tue, 15 Sep 2026, NeilBrown wrote:
> > > On Mon, 14 Sep 2026, Mateusz Guzik wrote:
> > > > Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
> > > > ---
> > > >  fs/namei.c           | 29 +++++++++++++++++++++++++++++
> > > >  include/linux/path.h | 14 ++++++++++++--
> > > >  2 files changed, 41 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fs/namei.c b/fs/namei.c
> > > > index ca4f5e3be99a..c8754c7ba8be 100644
> > > > --- a/fs/namei.c
> > > > +++ b/fs/namei.c
> > > > @@ -698,6 +698,35 @@ static __always_inline int lookup_inode_permission_may_exec(struct mnt_idmap *id
> > > >     return security_inode_permission(inode, mask);
> > > >  }
> > > >
> > > > +/**
> > > > + * path_create - WRITEME
> > > > + * @path: path to get the reference to
> > > > + * @mnt: WRITEME
> > > > + * @dentry: WRITEME
> > > > + *
> > > > + * Create a path object using the given vfsmount and dentry pair while incrementing
> > > > + * the reference count on both.
> > > > + */
> > > > +void path_create(struct path *path, struct vfsmount *mnt, struct dentry *dentry)
> > > > +{
> > > > +   path->mnt = mntget(mnt);
> > > > +   path->dentry = dget(dentry);
> > > > +}
> > > > +EXPORT_SYMBOL(path_create);
> > > > +
> > > > +/**
> > > > + * path_clone - WRITEME
> > > > + * @path: path to get the reference to
> > > > + *
> > > > + * Given a path increment the reference count to the dentry and the vfsmount.
> > > > + */
> > > > +void path_clone(const struct path *src, struct path *dst)
> > > > +{
> > > > +   dst->mnt = mntget(src->mnt);
> > > > +   dst->dentry = dget(src->dentry);
> > > > +}
> > > > +EXPORT_SYMBOL(path_clone);
> > > > +
> > > >  /**
> > > >   * path_get - get a reference to a path
> > > >   * @path: path to get the reference to
> > > > diff --git a/include/linux/path.h b/include/linux/path.h
> > > > index 7ea389dc764b..8a621e0e1921 100644
> > > > --- a/include/linux/path.h
> > > > +++ b/include/linux/path.h
> > > > @@ -10,8 +10,18 @@ struct path {
> > > >     struct dentry *dentry;
> > > >  } __randomize_layout;
> > > >
> > > > -extern void path_get(const struct path *);
> > > > -extern void path_put(const struct path *);
> > > > +void path_create(struct path *, struct vfsmount *, struct dentry *);
> > > > +void path_clone(const struct path *, struct path *);
> > > > +void path_get(const struct path *);
> > > > +void path_put(const struct path *);
> > > > +
> > > > +static inline void path_move(struct path *src, struct path *dst)
> > >
> > > Oh ...  this is backwards.  At least is it backwards compared to
> > > memmove().  I understand different people will read "move" differently
> > > and would prefer to avoid it.
> > > Can we do without it?  You only seem to use it in one place.
> > >
> > > How would you feel about:
> > >
> > >  struct path path_move_from(struct path *src)
> > >  {
> > >       struct path ret = *src;
> > >       src->mnt = NULL;
> > >       src->dentry = NULL;
> > >       return ret;
> > >  }
> > >
> > > and then use
> > >
> > >  file->__f_path = path_move_from(path);
> > >
> > > ??
> >
> > And then of course there is path_clone() which looks backwards to me.
> > Can we just change path_get() to return the path, like a lot of
> > FOO_get() functions do, and use
> >    newpath = path_clone(old_path);
> > ??
> 
> Apologies, I got busy with other stuff and completely forgot about
> this patchset.

Yes, life happens - sorry for my delay, I nearly forgot :-(

> 
> My issue with the proposed approach is that it prevents the routine
> from taking a look at the target.
> 
> While the content can be considered uninitialized (and thus arbitrary)
> at first, the code can be hacked up so that when the kernel is
> compiled with CONFIG_DEBUG_VFS the struct is extended with a magic
> number denoting it is populated. Then a repeat call to populate it can
> produce a splat.

I certainly see the value in these sorts of checks, but I don't think
they should be added by instrumenting the C code.  Rather it is a job
for a static analyser such as smatch.
There are lots and lots of places in the kernel where we assign a
pointer to a variable.  Changing all those to ptr_assign(var, val)
so that we can redefine ptr_assign() to check that var is NULL
would be horrible.
You seem to be suggesting doing exactly that for struct path, which is
simply 2 pointers.

I don't think the cost is worth it, when a similar effect can be
achieved in other ways.

> 
> I'm not married to "clone" and "move" terminology, which I only used
> because of Rust.
> 
> Would "path_dup" and "path_transfer" be clear enough for you as far as
> direction goes?
> 

It's not about the names, it is the design.
Any function which is given two pointers of the same type and has a
purpose of moving/copying/cloning from one to the other hits the same
problem: which direction does it go: left-to-right or right-to-left?
Some people will expect one, other will expect the other.  It is no-win.
I would always assume the same order as strcpy(), but others may not.

Contrast with
   foo = xxx(bar)

Here there is no ambiguity at all.

Thanks,
NeilBrown