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

[Xi Ruoyao <xry111@xry111.site>]

[-- 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