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@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University
From 16d02a1c44e5eed2ef2a2cc3220d0a74b35df822 Mon Sep 17 00:00:00 2001
From: Xi Ruoyao <xry111@xxxxxxxxxxx>
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@xxxxxxxxxxx>
---
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
