On Sun, Jun 23, 2024 at 3:22 AM Xi Ruoyao <xry111@xxxxxxxxxxx> wrote: > > On Sun, 2024-06-23 at 03:07 +0200, Mateusz Guzik wrote: > > On Sun, Jun 23, 2024 at 2:59 AM Xi Ruoyao <xry111@xxxxxxxxxxx> wrote: > > > > > > On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > > > > > > > I do think that we should make AT_EMPTY_PATH with a NULL path > > > > "JustWork(tm)", because the stupid "look if the pathname is empty" is > > > > horrible. > > > > > > > > But moving that check into getname() is *NOT* the right answer, > > > > because by the time you get to getname(), you have already lost. > > > > > > Oops. I'll try to get around of getname() too... > > > > > > > So the short-cut in vfs_fstatat() to never get a pathname is > > > > disgusting - people should have used 'fstat()' - but it's _important_ > > > > disgusting. > > > > > > The problem is we don't have fstat() for LoongArch, and it'll be > > > unusable on all 32-bit arch after 2037. > > > > > > And Arnd hates the idea adding fstat() for LoongArch because there would > > > be one more 32-bit arch broken in 2037. > > > > > > Or should we just add something like "fstat_2037()"? > > > > > > > In that case fstat is out of the question, but no problem. > > > > It was suggested to make AT_EMPTY_PATH + NULL pathname do the right > > thing and have the syscalls short-circuit as needed. > > > > for statx it would look like this (except you are going to have > > implement do_statx_by_fd): > > > > diff --git a/fs/stat.c b/fs/stat.c > > index 16aa1f5ceec4..0afe72b320cc 100644 > > --- a/fs/stat.c > > +++ b/fs/stat.c > > @@ -710,6 +710,9 @@ SYSCALL_DEFINE5(statx, > > int ret; > > struct filename *name; > > > > + if (flags == AT_EMPTY_PATH && filename == NULL) > > + return do_statx_by_fd(...); > > + > > name = getname_flags(filename, getname_statx_lookup_flags(flags)); > > ret = do_statx(dfd, name, flags, mask, buffer); > > putname(name); > > > > and so on > > > > Personally I would prefer if fstatx was added instead, fwiw most > > massaging in the area will be the same regardless. > > I do agree. But if we do it *now* would it be "breaking the userspace" > if some stupid program relies on fstatx() to return some error when the > path is NULL? The "stupid programs" may just exist in the wild... > You mean statx? fstatx would not accept a path to begin with. Worry about some code breaking is why I suggested a dedicated flag (AT_NO_PATH) myself in case fstatx is a no-go. I am not convinced messing with AT_* flags is justified to begin with. Any syscall which does not have a fd-only variant and is found to be routinely used with AT_EMPTY_PATH should get one instead. As far as I know that's only stat(due to a perf bug in glibc, now fixed) and increasingly statx. Suppose AT_EMPTY_PATH + NULL are to land and stat + statx get the treatment. What about all the other syscalls? Sorting all that out is quite a big of churn which is probably not worth it. But then there is a feature gap in that they EFAULT for this pair while the stat* ones don't and that's bound to raise confusion. Then one could add the check in the bowels of path lookup in similar way you do did to maintain the same behavior (but without per-syscall churn) and a big fat warning that anyone getting there often needs to get patched with short-circuiting the entire thing. So I think that's either a lot of churn or nasty additions. Regardless, as noted above, either making fstatx a thing or short-circuiting mostly the same patching has to be done for statx-related stuff. However, this is not my call to make. -- Mateusz Guzik <mjguzik gmail.com>
