Re: [RFC][PATCH] getname_maybe_null() - the third variant of pathname copy-in

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

On Wed, Oct 16, 2024 at 04:49:48PM +0200, Christian Brauner wrote:
> On Wed, Oct 16, 2024 at 03:00:50PM +0100, Al Viro wrote:
> > On Wed, Oct 16, 2024 at 10:32:16AM +0200, Christian Brauner wrote:
> > 
> > > > ended up calling user_path_at() with empty pathname and nothing like LOOKUP_EMPTY
> > > > in lookup_flags.  Which bails out with -ENOENT, since getname() in there does
> > > > so.  My variant bails out with -EBADF and I'd argue that neither is correct.
> > > > 
> > > > Not sure what's the sane solution here, need to think for a while...
> > > 
> > > Fwiw, in the other thread we concluded to just not care about AT_FDCWD with "".
> > > And so far I agree with that.
> > 
> > Subject:?
> 
> https://lore.kernel.org/r/CAGudoHHdccL5Lh8zAO-0swqqRCW4GXMSXhq4jQGoVj=UdBK-Lg@xxxxxxxxxxxxxx
> 
> Hm, this only speaks about the NULL case.
> 
> 
> I just looked through codesearch on github and on debian and the only
> example I found was
> https://sources.debian.org/src/snapd/2.65.3-1/tests/main/seccomp-statx/test-snapd-statx/bin/statx.py/?hl=71#L71
> 
> So really, just special-case it for statx() imho instead of spreading
> that ugliness everywhere?

Not sure, TBH.  I wonder if it would be simpler to have filename_lookup()
accept NULL for pathname and have it treat that as "".

Look: all it takes is the following trick
	* add const char *pathname to struct nameidata
	* in __set_nameidata() add
        p->pathname = likely(name) ? name->name : "";
	* in path_init() replace
	const char *s = nd->name->name;
with
	const char *s = nd->pathname;
and we are done.  Oh, and teach putname() to treat NULL as no-op.

With such changes in fs/namei.c we could do
        struct filename *name = getname_maybe_null(filename, flags);

	if (!name && dfd != AT_FDCWD)
		return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);
	ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS);
	putname(name);
	return ret;
in statx(2) and similar in vfs_fstatat().

With that I'm not even sure we want to bother with separate
vfs_statx_fd() - the overhead might very well turn out to
be tolerable.  It is non-zero, but that's a fairly straight
trip through filename_lookup() machinery.  Would require some
profiling, though...