[PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

Chromium sandbox apparently wants to deny statx [1] so it could properly
inspect arguments after the sandboxed process later falls back to fstat.
Because there's currently not a "fd-only" version of statx, so that the
sandbox has no way to ensure the path argument is empty without being
able to peek into the sandboxed process's memory. For architectures able
to do newfstatat though, glibc falls back to newfstatat after getting
-ENOSYS for statx, then the respective SIGSYS handler [2] takes care of
inspecting the path argument, transforming allowed newfstatat's into
fstat instead which is allowed and has the same type of return value.

But, as LoongArch is the first architecture to not have fstat nor
newfstatat, the LoongArch glibc does not attempt falling back at all
when it gets -ENOSYS for statx -- and you see the problem there!

Actually, back when the LoongArch port was under review, people were
aware of the same problem with sandboxing clone3 [3], so clone was
eventually kept. Unfortunately it seemed at that time no one had noticed
statx, so besides restoring fstat/newfstatat to LoongArch uapi (and
postponing the problem further), it seems inevitable that we would need
to tackle seccomp deep argument inspection.

However, this is obviously a decision that shouldn't be taken lightly,
so we just restore fstat/newfstatat by defining __ARCH_WANT_NEW_STAT
in unistd.h. This is the simplest solution for now, and so we hope the
community will tackle the long-standing problem of seccomp deep argument
inspection in the future [4][5].

More infomation please reading this thread [6].

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2823150
[2] https://chromium.googlesource.com/chromium/src/sandbox/+/c085b51940bd/linux/seccomp-bpf-helpers/sigsys_handlers.cc#355
[3] https://lore.kernel.org/linux-arch/20220511211231.GG7074@brightrain.aerifal.cx/
[4] https://lwn.net/Articles/799557/
[5] https://lpc.events/event/4/contributions/560/attachments/397/640/deep-arg-inspection.pdf
[6] https://lore.kernel.org/loongarch/20240226-granit-seilschaft-eccc2433014d@brauner/T/#t

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 arch/loongarch/include/uapi/asm/unistd.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/loongarch/include/uapi/asm/unistd.h b/arch/loongarch/include/uapi/asm/unistd.h
index fcb668984f03..b344b1f91715 100644
--- a/arch/loongarch/include/uapi/asm/unistd.h
+++ b/arch/loongarch/include/uapi/asm/unistd.h
@@ -1,4 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_SYS_CLONE
 #define __ARCH_WANT_SYS_CLONE3
 
-- 
2.43.0

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Sat, May 11, 2024, at 12:01, Huacai Chen wrote:
> Chromium sandbox apparently wants to deny statx [1] so it could properly
> inspect arguments after the sandboxed process later falls back to fstat.
> Because there's currently not a "fd-only" version of statx, so that the
> sandbox has no way to ensure the path argument is empty without being
> able to peek into the sandboxed process's memory. For architectures able
> to do newfstatat though, glibc falls back to newfstatat after getting
> -ENOSYS for statx, then the respective SIGSYS handler [2] takes care of
> inspecting the path argument, transforming allowed newfstatat's into
> fstat instead which is allowed and has the same type of return value.
>
> But, as LoongArch is the first architecture to not have fstat nor
> newfstatat, the LoongArch glibc does not attempt falling back at all
> when it gets -ENOSYS for statx -- and you see the problem there!

My main objection here is that this is inconsistent with 32-bit
architectures: we normally have newfstatat() on 64-bit
architectures but fstatat64() on 32-bit ones. While loongarch64
is the first 64-bit one that is missing newfstatat(), we have
riscv32 already without fstatat64().

Importantly, we can't just add fstatat64() on riscv32 because
there is no time64 version for it other than statx(), and I don't
want the architectures to diverge more than necessary.
I would not mind adding a variant of statx() that works for
both riscv32 and loongarch64 though, if it gets added to all
architectures.

      Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

Hi, Arnd,

On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, May 11, 2024, at 12:01, Huacai Chen wrote:
> > Chromium sandbox apparently wants to deny statx [1] so it could properly
> > inspect arguments after the sandboxed process later falls back to fstat.
> > Because there's currently not a "fd-only" version of statx, so that the
> > sandbox has no way to ensure the path argument is empty without being
> > able to peek into the sandboxed process's memory. For architectures able
> > to do newfstatat though, glibc falls back to newfstatat after getting
> > -ENOSYS for statx, then the respective SIGSYS handler [2] takes care of
> > inspecting the path argument, transforming allowed newfstatat's into
> > fstat instead which is allowed and has the same type of return value.
> >
> > But, as LoongArch is the first architecture to not have fstat nor
> > newfstatat, the LoongArch glibc does not attempt falling back at all
> > when it gets -ENOSYS for statx -- and you see the problem there!
>
> My main objection here is that this is inconsistent with 32-bit
> architectures: we normally have newfstatat() on 64-bit
> architectures but fstatat64() on 32-bit ones. While loongarch64
> is the first 64-bit one that is missing newfstatat(), we have
> riscv32 already without fstatat64().
Then how to move forward? Xuerui said that he wants to improve
seccomp, but a long time has already passed. And I think we should
solve this problem before Debian loong64 ports become usable.

>
> Importantly, we can't just add fstatat64() on riscv32 because
> there is no time64 version for it other than statx(), and I don't
> want the architectures to diverge more than necessary.
> I would not mind adding a variant of statx() that works for
> both riscv32 and loongarch64 though, if it gets added to all
> architectures.
As far as I know, Ren Guo is trying to implement riscv64 kernel +
riscv32 userspace, so I think riscv32 kernel won't be widely used?

Huacai
>
>       Arnd
>

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
> On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:

>> Importantly, we can't just add fstatat64() on riscv32 because
>> there is no time64 version for it other than statx(), and I don't
>> want the architectures to diverge more than necessary.
>> I would not mind adding a variant of statx() that works for
>> both riscv32 and loongarch64 though, if it gets added to all
>> architectures.
>
> As far as I know, Ren Guo is trying to implement riscv64 kernel +
> riscv32 userspace, so I think riscv32 kernel won't be widely used?

I was talking about the ABI, so it doesn't actually matter
what the kernel is: any userspace ABI without
CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
this is the only allowed configuration, while on others (arm32
or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
both 32-bit kernel and on 64-bit kernels with compat mode.

     Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
> > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> >> Importantly, we can't just add fstatat64() on riscv32 because
> >> there is no time64 version for it other than statx(), and I don't
> >> want the architectures to diverge more than necessary.
> >> I would not mind adding a variant of statx() that works for
> >> both riscv32 and loongarch64 though, if it gets added to all
> >> architectures.
> >
> > As far as I know, Ren Guo is trying to implement riscv64 kernel +
> > riscv32 userspace, so I think riscv32 kernel won't be widely used?
>
> I was talking about the ABI, so it doesn't actually matter
> what the kernel is: any userspace ABI without
> CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
> this is the only allowed configuration, while on others (arm32
> or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
> both 32-bit kernel and on 64-bit kernels with compat mode.
I don't know too much detail, but I think riscv32 can do something
similar to arm32 and x86-32, or we can wait for Xuerui to improve
seccomp. But there is no much time for loongarch because the Debian
loong64 port is coming soon.

Huacai

>
>      Arnd
>

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Sun, May 12, 2024, at 05:11, Huacai Chen wrote:
> On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
>> > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
>> this is the only allowed configuration, while on others (arm32
>> or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
>> both 32-bit kernel and on 64-bit kernels with compat mode.
> I don't know too much detail, but I think riscv32 can do something
> similar to arm32 and x86-32, or we can wait for Xuerui to improve
> seccomp. But there is no much time for loongarch because the Debian
> loong64 port is coming soon.

What I meant is that the other architectures only work by
accident if COMPAT_32BIT_TIME is enabled and statx() gets
blocked, but then they truncate the timestamps to the tim32
range, which is not acceptable behavior. Actually mips64 is
in the same situation because it also only supports 32-bit
timestamps in newstatat(), despite being a 64-bit
architecture with a 64-bit time_t in all other syscalls.

      Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

Hi, Arnd,

On Sun, May 12, 2024 at 3:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, May 12, 2024, at 05:11, Huacai Chen wrote:
> > On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
> >> > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
> >> this is the only allowed configuration, while on others (arm32
> >> or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
> >> both 32-bit kernel and on 64-bit kernels with compat mode.
> > I don't know too much detail, but I think riscv32 can do something
> > similar to arm32 and x86-32, or we can wait for Xuerui to improve
> > seccomp. But there is no much time for loongarch because the Debian
> > loong64 port is coming soon.
>
> What I meant is that the other architectures only work by
> accident if COMPAT_32BIT_TIME is enabled and statx() gets
> blocked, but then they truncate the timestamps to the tim32
> range, which is not acceptable behavior. Actually mips64 is
> in the same situation because it also only supports 32-bit
> timestamps in newstatat(), despite being a 64-bit
> architecture with a 64-bit time_t in all other syscalls.
We can only wait for the seccomp side to be fixed now? Or we can get
this patch upstream for LoongArch64 at the moment, and wait for
seccomp to fix RISCV32 (and LoongArch32) in future?

Huacai

>
>       Arnd
>

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Xi Ruoyao]

On Sat, 2024-06-15 at 16:52 +0800, Huacai Chen wrote:
> Hi, Arnd,
> 
> On Sun, May 12, 2024 at 3:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > On Sun, May 12, 2024, at 05:11, Huacai Chen wrote:
> > > On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
> > > > > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
> > > > this is the only allowed configuration, while on others (arm32
> > > > or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
> > > > both 32-bit kernel and on 64-bit kernels with compat mode.
> > > I don't know too much detail, but I think riscv32 can do something
> > > similar to arm32 and x86-32, or we can wait for Xuerui to improve
> > > seccomp. But there is no much time for loongarch because the Debian
> > > loong64 port is coming soon.
> > 
> > What I meant is that the other architectures only work by
> > accident if COMPAT_32BIT_TIME is enabled and statx() gets
> > blocked, but then they truncate the timestamps to the tim32
> > range, which is not acceptable behavior. Actually mips64 is
> > in the same situation because it also only supports 32-bit
> > timestamps in newstatat(), despite being a 64-bit
> > architecture with a 64-bit time_t in all other syscalls.
> We can only wait for the seccomp side to be fixed now? Or we can get
> this patch upstream for LoongArch64 at the moment, and wait for
> seccomp to fix RISCV32 (and LoongArch32) in future?

I'm wondering why not just introduce a new syscall or extend statx with
a new flag, as we've discussed many times.  They have their own
disadvantages but better than this, IMO.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

On Sat, Jun 15, 2024 at 4:55 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sat, 2024-06-15 at 16:52 +0800, Huacai Chen wrote:
> > Hi, Arnd,
> >
> > On Sun, May 12, 2024 at 3:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Sun, May 12, 2024, at 05:11, Huacai Chen wrote:
> > > > On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
> > > > > > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
> > > > > this is the only allowed configuration, while on others (arm32
> > > > > or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
> > > > > both 32-bit kernel and on 64-bit kernels with compat mode.
> > > > I don't know too much detail, but I think riscv32 can do something
> > > > similar to arm32 and x86-32, or we can wait for Xuerui to improve
> > > > seccomp. But there is no much time for loongarch because the Debian
> > > > loong64 port is coming soon.
> > >
> > > What I meant is that the other architectures only work by
> > > accident if COMPAT_32BIT_TIME is enabled and statx() gets
> > > blocked, but then they truncate the timestamps to the tim32
> > > range, which is not acceptable behavior. Actually mips64 is
> > > in the same situation because it also only supports 32-bit
> > > timestamps in newstatat(), despite being a 64-bit
> > > architecture with a 64-bit time_t in all other syscalls.
> > We can only wait for the seccomp side to be fixed now? Or we can get
> > this patch upstream for LoongArch64 at the moment, and wait for
> > seccomp to fix RISCV32 (and LoongArch32) in future?
>
> I'm wondering why not just introduce a new syscall or extend statx with
> a new flag, as we've discussed many times.  They have their own
> disadvantages but better than this, IMO.
We should move things forward, in any way. :)

Huacai

>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Sat, Jun 15, 2024, at 11:29, Huacai Chen wrote:
> On Sat, Jun 15, 2024 at 4:55 PM Xi Ruoyao <xry111@xry111.site> wrote:
>>
>> On Sat, 2024-06-15 at 16:52 +0800, Huacai Chen wrote:
>> > Hi, Arnd,
>> >
>> > On Sun, May 12, 2024 at 3:53 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> > >
>> > > On Sun, May 12, 2024, at 05:11, Huacai Chen wrote:
>> > > > On Sat, May 11, 2024 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> > > > > On Sat, May 11, 2024, at 16:28, Huacai Chen wrote:
>> > > > > > On Sat, May 11, 2024 at 8:17 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> > > > > CONFIG_COMPAT_32BIT_TIME is equally affected here. On riscv32
>> > > > > this is the only allowed configuration, while on others (arm32
>> > > > > or x86-32 userland) you can turn off COMPAT_32BIT_TIME on
>> > > > > both 32-bit kernel and on 64-bit kernels with compat mode.
>> > > > I don't know too much detail, but I think riscv32 can do something
>> > > > similar to arm32 and x86-32, or we can wait for Xuerui to improve
>> > > > seccomp. But there is no much time for loongarch because the Debian
>> > > > loong64 port is coming soon.
>> > >
>> > > What I meant is that the other architectures only work by
>> > > accident if COMPAT_32BIT_TIME is enabled and statx() gets
>> > > blocked, but then they truncate the timestamps to the tim32
>> > > range, which is not acceptable behavior. Actually mips64 is
>> > > in the same situation because it also only supports 32-bit
>> > > timestamps in newstatat(), despite being a 64-bit
>> > > architecture with a 64-bit time_t in all other syscalls.
>> > We can only wait for the seccomp side to be fixed now? Or we can get
>> > this patch upstream for LoongArch64 at the moment, and wait for
>> > seccomp to fix RISCV32 (and LoongArch32) in future?
>>
>> I'm wondering why not just introduce a new syscall or extend statx with
>> a new flag, as we've discussed many times.  They have their own
>> disadvantages but better than this, IMO.
> We should move things forward, in any way. :)

Wouldn't it be sufficient to move the AT_EMPTY_PATH hack
from vfs_fstatat() to vfs_statx() so we can make them
behave the same way?

As far as I can tell, the only difference between the two is
that fstatat64() and similar already has added the check for
zero-length strings in order to make using vfs_fstatat()
fast and safe when called from glibc stat().

     Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Xi Ruoyao]

On Sat, 2024-06-15 at 13:47 +0200, Arnd Bergmann wrote:

/* snip */

> > > > We can only wait for the seccomp side to be fixed now? Or we can get
> > > > this patch upstream for LoongArch64 at the moment, and wait for
> > > > seccomp to fix RISCV32 (and LoongArch32) in future?
> > > 
> > > I'm wondering why not just introduce a new syscall or extend statx with
> > > a new flag, as we've discussed many times.  They have their own
> > > disadvantages but better than this, IMO.
> > We should move things forward, in any way. :)
> 
> Wouldn't it be sufficient to move the AT_EMPTY_PATH hack
> from vfs_fstatat() to vfs_statx() so we can make them
> behave the same way?
> 
> As far as I can tell, the only difference between the two is
> that fstatat64() and similar already has added the check for
> zero-length strings in order to make using vfs_fstatat()
> fast and safe when called from glibc stat().

Do you mean https://git.kernel.org/torvalds/c/9013c51c630a?  It (only
partially) fix the performance issue but it won't help seccomp.  The
problem is you cannot check if the string is zero-length with seccomp.
Thus seccomp cannot audit fstatat properly as well.

In [Firefox] *all* fstatat (and statx) calls are trapped and *the signal
handler* audit this fstatat call.  If flags & AT_EMPTY_PATH and path is
zero-length, it calls fstat to do the job.  But on LoongArch there is no
way to "do the job" as the only stat-family call is statx.

[Firefox]:https://searchfox.org/mozilla-central/source/security/sandbox/linux/SandboxFilter.cpp#364

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
> On Sat, 2024-06-15 at 13:47 +0200, Arnd Bergmann wrote:
> 
> /* snip */
> 
> > > > > We can only wait for the seccomp side to be fixed now? Or we can get
> > > > > this patch upstream for LoongArch64 at the moment, and wait for
> > > > > seccomp to fix RISCV32 (and LoongArch32) in future?
> > > > 
> > > > I'm wondering why not just introduce a new syscall or extend statx with
> > > > a new flag, as we've discussed many times.  They have their own
> > > > disadvantages but better than this, IMO.
> > > We should move things forward, in any way. :)
> > 
> > Wouldn't it be sufficient to move the AT_EMPTY_PATH hack
> > from vfs_fstatat() to vfs_statx() so we can make them
> > behave the same way?
> > 
> > As far as I can tell, the only difference between the two is
> > that fstatat64() and similar already has added the check for
> > zero-length strings in order to make using vfs_fstatat()
> > fast and safe when called from glibc stat().
> 
> Do you mean https://git.kernel.org/torvalds/c/9013c51c630a?  It (only
> partially) fix the performance issue but it won't help seccomp.  The
> problem is you cannot check if the string is zero-length with seccomp.
> Thus seccomp cannot audit fstatat properly as well.
> 
> In [Firefox] *all* fstatat (and statx) calls are trapped and *the signal
> handler* audit this fstatat call.  If flags & AT_EMPTY_PATH and path is
> zero-length, it calls fstat to do the job.  But on LoongArch there is no
> way to "do the job" as the only stat-family call is statx.
> 
> [Firefox]:https://searchfox.org/mozilla-central/source/security/sandbox/linux/SandboxFilter.cpp#364

Just spent some brain cycles to make a quick hack adding a new statx
flag.  Patch attached.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0001-RFC-vfs-Add-AT_FORCE_EMPTY_PATH.patch --]
[-- Type: text/x-patch, Size: 3908 bytes --]

From 16d02a1c44e5eed2ef2a2cc3220d0a74b35df822 Mon Sep 17 00:00:00 2001
From: Xi Ruoyao <xry111@xry111.site>
Date: Sat, 15 Jun 2024 20:44:04 +0800
Subject: [PATCH] RFC: vfs: Add AT_FORCE_EMPTY_PATH

It behaves as if AT_EMPTY_PATH with an empty path (the input path will
be ignored).

It's better than AT_EMPTY_PATH for implementing fstat with statx (it's
needed after 2037 for 32-bit systems) because there's no need to copy
from user, and it's auditable by seccomp (though personally I'm really
not a fan if seccomp).

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 fs/namei.c                 | 8 +++++++-
 fs/stat.c                  | 4 +++-
 include/linux/namei.h      | 4 ++++
 include/trace/misc/fs.h    | 1 +
 include/uapi/linux/fcntl.h | 3 +++
 5 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa09a..2f012ec8f072 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -147,7 +147,13 @@ getname_flags(const char __user *filename, int flags, int *empty)
 	kname = (char *)result->iname;
 	result->name = kname;
 
-	len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
+	if (!(flags & LOOKUP_FORCE_EMPTY))
+		len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
+	else {
+		len = 0;
+		kname[0] = '\0';
+	}
+
 	if (unlikely(len < 0)) {
 		__putname(result);
 		return ERR_PTR(len);
diff --git a/fs/stat.c b/fs/stat.c
index 70bd3e888cfa..be81fc12bd3a 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -210,6 +210,8 @@ int getname_statx_lookup_flags(int flags)
 		lookup_flags |= LOOKUP_AUTOMOUNT;
 	if (flags & AT_EMPTY_PATH)
 		lookup_flags |= LOOKUP_EMPTY;
+	if (flags & AT_FORCE_EMPTY_PATH)
+		lookup_flags |= LOOKUP_EMPTY | LOOKUP_FORCE_EMPTY;
 
 	return lookup_flags;
 }
@@ -237,7 +239,7 @@ static int vfs_statx(int dfd, struct filename *filename, int flags,
 	int error;
 
 	if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT | AT_EMPTY_PATH |
-		      AT_STATX_SYNC_TYPE))
+		      AT_STATX_SYNC_TYPE | AT_FORCE_EMPTY_PATH))
 		return -EINVAL;
 
 retry:
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 967aa9ea9f96..d19e5166101b 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -45,9 +45,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
 #define LOOKUP_IN_ROOT		0x100000 /* Treat dirfd as fs root. */
 #define LOOKUP_CACHED		0x200000 /* Only do cached lookup */
 #define LOOKUP_LINKAT_EMPTY	0x400000 /* Linkat request with empty path. */
+
 /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
 #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
 
+/* If this is set, LOOKUP_EMPTY must be set as well. */
+#define LOOKUP_FORCE_EMPTY	0x800000 /* Consider path empty. */
+
 extern int path_pts(struct path *path);
 
 extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
diff --git a/include/trace/misc/fs.h b/include/trace/misc/fs.h
index 738b97f22f36..46489426f18a 100644
--- a/include/trace/misc/fs.h
+++ b/include/trace/misc/fs.h
@@ -119,4 +119,5 @@
 		{ LOOKUP_NO_XDEV,	"NO_XDEV" }, \
 		{ LOOKUP_BENEATH,	"BENEATH" }, \
 		{ LOOKUP_IN_ROOT,	"IN_ROOT" }, \
+		{ LOOKUP_FORCE_EMPTY,	"FORCE_EMPTY" }, \
 		{ LOOKUP_CACHED,	"CACHED" })
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index c0bcc185fa48..71d3dc92c86e 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -113,6 +113,9 @@
 #define AT_STATX_DONT_SYNC	0x4000	/* - Don't sync attributes with the server */
 
 #define AT_RECURSIVE		0x8000	/* Apply to the entire subtree */
+#define AT_FORCE_EMPTY_PATH	0x10000	/* Ignore path and behave as if
+                                           AT_EMPTY_PATH is set and path
+                                           is empty */
 
 /* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits... */
 #define AT_HANDLE_FID		AT_REMOVEDIR	/* file handle is needed to
-- 
2.45.2

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Sat, Jun 15, 2024, at 15:12, Xi Ruoyao wrote:
> On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
>> 
>> [Firefox]:https://searchfox.org/mozilla-central/source/security/sandbox/linux/SandboxFilter.cpp#364
>
> Just spent some brain cycles to make a quick hack adding a new statx
> flag.  Patch attached.
>

Thanks for the prototype. I agree that this is not a good API
but that it would address the issue and I am fine with merging
something like this if you can convince the VFS maintainers.

      Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Xi Ruoyao]

On Mon, 2024-06-17 at 08:35 +0200, Arnd Bergmann wrote:
> On Sat, Jun 15, 2024, at 15:12, Xi Ruoyao wrote:
> > On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
> > > 
> > > [Firefox]:
> > > https://searchfox.org/mozilla-central/source/security/sandbox/linu
> > > x/SandboxFilter.cpp#364
> > 
> > Just spent some brain cycles to make a quick hack adding a new statx
> > flag.  Patch attached.
> > 
> 
> Thanks for the prototype. I agree that this is not a good API

What is particular bad with it?  Maybe we can improve before annoying
VFS guys :).

> but that it would address the issue and I am fine with merging
> something like this if you can convince the VFS maintainers.

Before that I'd like someone to purpose a better name.  I really dislike
"AT_FORCE_EMPTY_PATH" but I cannot come up with something better.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Mon, Jun 17, 2024, at 08:45, Xi Ruoyao wrote:
> On Mon, 2024-06-17 at 08:35 +0200, Arnd Bergmann wrote:
>> On Sat, Jun 15, 2024, at 15:12, Xi Ruoyao wrote:
>> > On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
>> > > 
>> > > [Firefox]:
>> > > https://searchfox.org/mozilla-central/source/security/sandbox/linu
>> > > x/SandboxFilter.cpp#364
>> > 
>> > Just spent some brain cycles to make a quick hack adding a new statx
>> > flag.  Patch attached.
>> > 
>> 
>> Thanks for the prototype. I agree that this is not a good API
>
> What is particular bad with it?  Maybe we can improve before annoying
> VFS guys :).

I can't come up with anything better either, the problem I
see is mainly that the man page has to explain both AT_EMPTY_PATH
and AT_FORCE_EMPTY_PATH, which are very similar for compatibility
reasons only. We would clearly not design a new interface to have
both, but we can't change existing behavior either.

       Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Huacai Chen]

Hi, Ruoyao,

On Mon, Jun 17, 2024 at 2:45 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Mon, 2024-06-17 at 08:35 +0200, Arnd Bergmann wrote:
> > On Sat, Jun 15, 2024, at 15:12, Xi Ruoyao wrote:
> > > On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
> > > >
> > > > [Firefox]:
> > > > https://searchfox.org/mozilla-central/source/security/sandbox/linu
> > > > x/SandboxFilter.cpp#364
> > >
> > > Just spent some brain cycles to make a quick hack adding a new statx
> > > flag.  Patch attached.
> > >
> >
> > Thanks for the prototype. I agree that this is not a good API
>
> What is particular bad with it?  Maybe we can improve before annoying
> VFS guys :).
>
> > but that it would address the issue and I am fine with merging
> > something like this if you can convince the VFS maintainers.
>
> Before that I'd like someone to purpose a better name.  I really dislike
> "AT_FORCE_EMPTY_PATH" but I cannot come up with something better.
Any updates? Have you submitted this patch? I hope we can end up at 6.11. :)

Huacai

>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Xi Ruoyao]

On Sat, 2024-06-22 at 15:45 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
> 
> On Mon, Jun 17, 2024 at 2:45 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > On Mon, 2024-06-17 at 08:35 +0200, Arnd Bergmann wrote:
> > > On Sat, Jun 15, 2024, at 15:12, Xi Ruoyao wrote:
> > > > On Sat, 2024-06-15 at 20:12 +0800, Xi Ruoyao wrote:
> > > > > 
> > > > > [Firefox]:
> > > > > https://searchfox.org/mozilla-central/source/security/sandbox/linu
> > > > > x/SandboxFilter.cpp#364
> > > > 
> > > > Just spent some brain cycles to make a quick hack adding a new statx
> > > > flag.  Patch attached.
> > > > 
> > > 
> > > Thanks for the prototype. I agree that this is not a good API
> > 
> > What is particular bad with it?  Maybe we can improve before annoying
> > VFS guys :).
> > 
> > > but that it would address the issue and I am fine with merging
> > > something like this if you can convince the VFS maintainers.
> > 
> > Before that I'd like someone to purpose a better name.  I really dislike
> > "AT_FORCE_EMPTY_PATH" but I cannot come up with something better.
> Any updates? Have you submitted this patch? I hope we can end up at 6.11. :)

https://lore.kernel.org/linux-fsdevel/20240622105621.7922-1-xry111@xry111.site/


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[maobibo]

On 2024/5/11 下午8:17, Arnd Bergmann wrote:
> On Sat, May 11, 2024, at 12:01, Huacai Chen wrote:
>> Chromium sandbox apparently wants to deny statx [1] so it could properly
>> inspect arguments after the sandboxed process later falls back to fstat.
>> Because there's currently not a "fd-only" version of statx, so that the
>> sandbox has no way to ensure the path argument is empty without being
>> able to peek into the sandboxed process's memory. For architectures able
>> to do newfstatat though, glibc falls back to newfstatat after getting
>> -ENOSYS for statx, then the respective SIGSYS handler [2] takes care of
>> inspecting the path argument, transforming allowed newfstatat's into
>> fstat instead which is allowed and has the same type of return value.
>>
>> But, as LoongArch is the first architecture to not have fstat nor
>> newfstatat, the LoongArch glibc does not attempt falling back at all
>> when it gets -ENOSYS for statx -- and you see the problem there!
> 
> My main objection here is that this is inconsistent with 32-bit
> architectures: we normally have newfstatat() on 64-bit
> architectures but fstatat64() on 32-bit ones. While loongarch64
> is the first 64-bit one that is missing newfstatat(), we have
> riscv32 already without fstatat64().
> 
> Importantly, we can't just add fstatat64() on riscv32 because
> there is no time64 version for it other than statx(), and I don't
> want the architectures to diverge more than necessary.
yes, I agree. Normally there is newfstatat() on 64-bit architectures but 
fstatat64() on 32-bit ones.

I do not understand why fstatat64() can be added for riscv32 still.
32bit timestamp seems works well for the present, it is valid until
(0x1UL << 32) / 365 / 24 / 3600 + 1970 == 2106 year. Year 2106 should
be enough for 32bit system.

Regards
Bibo Mao


> I would not mind adding a variant of statx() that works for
> both riscv32 and loongarch64 though, if it gets added to all
> architectures.
> 
>        Arnd
> 

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[Arnd Bergmann]

On Wed, May 15, 2024, at 09:30, maobibo wrote:
> On 2024/5/11 下午8:17, Arnd Bergmann wrote:
>> On Sat, May 11, 2024, at 12:01, Huacai Chen wrote:
>> 
>> Importantly, we can't just add fstatat64() on riscv32 because
>> there is no time64 version for it other than statx(), and I don't
>> want the architectures to diverge more than necessary.
> yes, I agree. Normally there is newfstatat() on 64-bit architectures but 
> fstatat64() on 32-bit ones.
>
> I do not understand why fstatat64() can be added for riscv32 still.
> 32bit timestamp seems works well for the present, it is valid until
> (0x1UL << 32) / 365 / 24 / 3600 + 1970 == 2106 year. Year 2106 should
> be enough for 32bit system.

There is a very small number of interfaces for which we ended up
not using a 64-bit time_t replacement, but those are only for
relative times, not epoch based offsets. The main problems
here are:

- time_t is defined to be a signed value in posix, and we need
  to handle file timestamps before 1970 in stat(), so changing
  this one to be unsigned is not an option.

- A lot of products have already shipped that will have to
  be supported past 2038 on existing 32-bit hardware. We
  cannot regress on architectures that have already been
  fixed to support this. 

- file timestamps can also be set into the future, so applications
  relying on this are broken before 2038.

      Arnd

Re: [PATCH] LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h

[maobibo]

On 2024/5/15 下午10:25, Arnd Bergmann wrote:
> On Wed, May 15, 2024, at 09:30, maobibo wrote:
>> On 2024/5/11 下午8:17, Arnd Bergmann wrote:
>>> On Sat, May 11, 2024, at 12:01, Huacai Chen wrote:
>>>
>>> Importantly, we can't just add fstatat64() on riscv32 because
>>> there is no time64 version for it other than statx(), and I don't
>>> want the architectures to diverge more than necessary.
>> yes, I agree. Normally there is newfstatat() on 64-bit architectures but
>> fstatat64() on 32-bit ones.
>>
>> I do not understand why fstatat64() can be added for riscv32 still.
>> 32bit timestamp seems works well for the present, it is valid until
>> (0x1UL << 32) / 365 / 24 / 3600 + 1970 == 2106 year. Year 2106 should
>> be enough for 32bit system.
> 
> There is a very small number of interfaces for which we ended up
> not using a 64-bit time_t replacement, but those are only for
> relative times, not epoch based offsets. The main problems
> here are:
> 
> - time_t is defined to be a signed value in posix, and we need
>    to handle file timestamps before 1970 in stat(), so changing
>    this one to be unsigned is not an option.
> 
> - A lot of products have already shipped that will have to
>    be supported past 2038 on existing 32-bit hardware. We
>    cannot regress on architectures that have already been
>    fixed to support this.
> 
> - file timestamps can also be set into the future, so applications
>    relying on this are broken before 2038.
I see. And thanks for detailed explanation.

Regards
Bibo Mao
> 
>        Arnd
>