On Tue, Oct 08, 2024 at 05:57:00AM +0200, Mateusz Guzik wrote:
> On Mon, Oct 7, 2024 at 3:08 PM Xi Ruoyao <xry111@xxxxxxxxxxx> wrote:
> >
> > We've supported {statx,fstatat}(real_fd, NULL, AT_EMPTY_PATH, ...) since
> > Linux 6.11 for better performance. However there are other cases, for
> > example using AT_FDCWD as the fd or having AT_SYMLINK_NOFOLLOW in flags,
> > not covered by the fast path. While it may be impossible, too
> > difficult, or not very beneficial to optimize these cases, we should
> > still turn NULL into "" for them in the slow path to make the API easier
> > to be documented and used.
> >
> > Fixes: 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, ...)")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Xi Ruoyao <xry111@xxxxxxxxxxx>
> > ---
> > fs/stat.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/stat.c b/fs/stat.c
> > index ed9d4fd8ba2c..5d1b51c23c62 100644
> > --- a/fs/stat.c
> > +++ b/fs/stat.c
> > @@ -337,8 +337,11 @@ int vfs_fstatat(int dfd, const char __user *filename,
> > flags &= ~AT_NO_AUTOMOUNT;
> > if (flags == AT_EMPTY_PATH && vfs_empty_path(dfd, filename))
> > return vfs_fstat(dfd, stat);
> > + else if ((flags & AT_EMPTY_PATH) && !filename)
> > + name = getname_kernel("");
> > + else
> > + name = getname_flags(filename, getname_statx_lookup_flags(statx_flags));
> >
> > - name = getname_flags(filename, getname_statx_lookup_flags(statx_flags));
> > ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS);
> > putname(name);
> >
> > @@ -791,8 +794,11 @@ SYSCALL_DEFINE5(statx,
> > lflags = flags & ~(AT_NO_AUTOMOUNT | AT_STATX_SYNC_TYPE);
> > if (lflags == AT_EMPTY_PATH && vfs_empty_path(dfd, filename))
> > return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);
> > + else if ((lflags & AT_EMPTY_PATH) && !filename)
> > + name = getname_kernel("");
> > + else
> > + name = getname_flags(filename, getname_statx_lookup_flags(flags));
> >
> > - name = getname_flags(filename, getname_statx_lookup_flags(flags));
> > ret = do_statx(dfd, name, flags, mask, buffer);
> > putname(name);
> >
>
> I thought you are going to patch up the 2 callsites of
> vfs_empty_path() or add the flags argument to said routine so that it
> can do the branching internally.
>
> Either way I don't think implementing AT_FDCWD + NULL + AT_EMPTY_PATH
> with getname_kernel("") is necessary.
Folks, please don't go there. Really. IMO vfs_empty_path() is a wrong API
in the first place. Too low-level and racy as well.
See the approach in #work.xattr; I'm going to lift that into fs/namei.c
(well, the slow path - everything after "if path is NULL, we are done") and
yes, fs/stat.c users get handled better that way.
