linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid()
@ 2023-03-04 14:28 Willy Tarreau
  2023-03-04 14:28 ` [PATCH 1/5] tools/nolibc: add getuid() and geteuid() Willy Tarreau
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

Hello Paul,

this is the second series of updates for nolibc. The work was
started by Feiyang Chen who got rid of some local definitions in
favor of inclusion of linux/fcntl.h. Consecutive to this I noticed
new issues related to redefinitions of S_IFDIR and friends and
figured that it can happen with some compilers that /usr/include is
inconditionally checked and causes annoyance around these flags that
are defined both in glibc and the kernel. The UAPI includes already
have a specific check for this, so the issue was fixed by always
including linux/stat.h in types.h and conditionally defining our own
flags like UAPI does.

Feiyang also added statx() because loongarch needs it for stat().

Finally I added getuid() and geteuid() so that we can finally skip
the two tests that require privileges in "run-user" native tests. Now
they appear as "SKIPPED" in the output, and the test is reported as
successful since there's no error anymore. I was finding it really
annoying to always have to carefully check errors that were expected
on success, now that's just bad old memories.

This series applies on top of Vincent's stdint series that was rebased
on top of your dev.2023.02.22a branch. Likewise, if it could make it
for 2.4 that would be great, especially since the next loongarch series
depends on it.

Thanks!
Willy

Feiyang Chen (2):
  tools/nolibc: Include linux/fcntl.h and remove duplicate code
  tools/nolibc: Add statx() and make stat() rely on statx() if necessary

Willy Tarreau (3):
  tools/nolibc: add getuid() and geteuid()
  selftests/nolibc: skip the chroot_root and link_dir tests when not
    privileged
  tools/nolibc: check for S_I* macros before defining them

 tools/include/nolibc/sys.h                   | 100 ++++++++++++++++++-
 tools/include/nolibc/types.h                 |  28 ++++--
 tools/testing/selftests/nolibc/nolibc-test.c |   8 +-
 3 files changed, 127 insertions(+), 9 deletions(-)

-- 
2.17.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/5] tools/nolibc: add getuid() and geteuid()
  2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
@ 2023-03-04 14:28 ` Willy Tarreau
  2023-03-04 14:28 ` [PATCH 2/5] selftests/nolibc: skip the chroot_root and link_dir tests when not privileged Willy Tarreau
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

This can be useful to avoid attempting some privileged operations,
starting from the nolibc-test tool that gets two failures when not
privileged.

We call getuid32() and geteuid32() when they are defined, and fall
back to getuid() and geteuid() otherwise.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/sys.h | 42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index b5f8cd35c03b..115579e7f1db 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -410,6 +410,27 @@ int getdents64(int fd, struct linux_dirent64 *dirp, int count)
 }
 
 
+/*
+ * uid_t geteuid(void);
+ */
+
+static __attribute__((unused))
+uid_t sys_geteuid(void)
+{
+#ifdef __NR_geteuid32
+	return my_syscall0(__NR_geteuid32);
+#else
+	return my_syscall0(__NR_geteuid);
+#endif
+}
+
+static __attribute__((unused))
+uid_t geteuid(void)
+{
+	return sys_geteuid();
+}
+
+
 /*
  * pid_t getpgid(pid_t pid);
  */
@@ -544,6 +565,27 @@ int gettimeofday(struct timeval *tv, struct timezone *tz)
 }
 
 
+/*
+ * uid_t getuid(void);
+ */
+
+static __attribute__((unused))
+uid_t sys_getuid(void)
+{
+#ifdef __NR_getuid32
+	return my_syscall0(__NR_getuid32);
+#else
+	return my_syscall0(__NR_getuid);
+#endif
+}
+
+static __attribute__((unused))
+uid_t getuid(void)
+{
+	return sys_getuid();
+}
+
+
 /*
  * int ioctl(int fd, unsigned long req, void *value);
  */
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/5] selftests/nolibc: skip the chroot_root and link_dir tests when not privileged
  2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
  2023-03-04 14:28 ` [PATCH 1/5] tools/nolibc: add getuid() and geteuid() Willy Tarreau
@ 2023-03-04 14:28 ` Willy Tarreau
  2023-03-04 14:28 ` [PATCH 3/5] tools/nolibc: check for S_I* macros before defining them Willy Tarreau
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

These two tests always fail when the program is started natively as an
unprivileged user, and require the user to carefully check the output
of "make run-user" and ignore them.

Let's add an euid check and condition these two tests to euid==0. Now
the test case stops needlessly reporting failures. E.g.:

  $ make -C tools/testing/selftests/nolibc run-user
  ...
    CC      nolibc-test
  123 test(s) passed.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 772f88bda0f1..6a7c13f0cd61 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -477,6 +477,7 @@ static int test_getpagesize(void)
 int run_syscall(int min, int max)
 {
 	struct stat stat_buf;
+	int euid0;
 	int proc;
 	int test;
 	int tmp;
@@ -486,6 +487,9 @@ int run_syscall(int min, int max)
 	/* <proc> indicates whether or not /proc is mounted */
 	proc = stat("/proc", &stat_buf) == 0;
 
+	/* this will be used to skip certain tests that can't be run unprivileged */
+	euid0 = geteuid() == 0;
+
 	for (test = min; test >= 0 && test <= max; test++) {
 		int llen = 0; // line length
 
@@ -511,7 +515,7 @@ int run_syscall(int min, int max)
 		CASE_TEST(chmod_net);         EXPECT_SYSZR(proc, chmod("/proc/self/net", 0555)); break;
 		CASE_TEST(chmod_self);        EXPECT_SYSER(proc, chmod("/proc/self", 0555), -1, EPERM); break;
 		CASE_TEST(chown_self);        EXPECT_SYSER(proc, chown("/proc/self", 0, 0), -1, EPERM); break;
-		CASE_TEST(chroot_root);       EXPECT_SYSZR(1, chroot("/")); break;
+		CASE_TEST(chroot_root);       EXPECT_SYSZR(euid0, chroot("/")); break;
 		CASE_TEST(chroot_blah);       EXPECT_SYSER(1, chroot("/proc/self/blah"), -1, ENOENT); break;
 		CASE_TEST(chroot_exe);        EXPECT_SYSER(proc, chroot("/proc/self/exe"), -1, ENOTDIR); break;
 		CASE_TEST(close_m1);          EXPECT_SYSER(1, close(-1), -1, EBADF); break;
@@ -536,7 +540,7 @@ int run_syscall(int min, int max)
 		CASE_TEST(ioctl_tiocinq);     EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break;
 		CASE_TEST(link_root1);        EXPECT_SYSER(1, link("/", "/"), -1, EEXIST); break;
 		CASE_TEST(link_blah);         EXPECT_SYSER(1, link("/proc/self/blah", "/blah"), -1, ENOENT); break;
-		CASE_TEST(link_dir);          EXPECT_SYSER(1, link("/", "/blah"), -1, EPERM); break;
+		CASE_TEST(link_dir);          EXPECT_SYSER(euid0, link("/", "/blah"), -1, EPERM); break;
 		CASE_TEST(link_cross);        EXPECT_SYSER(proc, link("/proc/self/net", "/blah"), -1, EXDEV); break;
 		CASE_TEST(lseek_m1);          EXPECT_SYSER(1, lseek(-1, 0, SEEK_SET), -1, EBADF); break;
 		CASE_TEST(lseek_0);           EXPECT_SYSER(1, lseek(0, 0, SEEK_SET), -1, ESPIPE); break;
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/5] tools/nolibc: check for S_I* macros before defining them
  2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
  2023-03-04 14:28 ` [PATCH 1/5] tools/nolibc: add getuid() and geteuid() Willy Tarreau
  2023-03-04 14:28 ` [PATCH 2/5] selftests/nolibc: skip the chroot_root and link_dir tests when not privileged Willy Tarreau
@ 2023-03-04 14:28 ` Willy Tarreau
  2023-03-04 14:28 ` [PATCH 4/5] tools/nolibc: Include linux/fcntl.h and remove duplicate code Willy Tarreau
  2023-03-04 14:28 ` [PATCH 5/5] tools/nolibc: Add statx() and make stat() rely on statx() if necessary Willy Tarreau
  4 siblings, 0 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

Defining S_I* flags in types.h can cause some build failures if
linux/stat.h is included prior to it. But if not defined, some toolchains
that include some glibc parts will in turn fail because linux/stat.h
already takes care of avoiding these definitions when glibc is present.

Let's preserve the macros here but first include linux/stat.h and check
for their definition before doing so. We also define the previously
missing permission macros so that we don't get a different behavior
depending on the first include found.

Cc: Feiyang Chen <chenfeiyang@loongson.cn>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/types.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index fbbc0e68c001..47a0997d2d74 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -9,6 +9,7 @@
 
 #include "std.h"
 #include <linux/time.h>
+#include <linux/stat.h>
 
 
 /* Only the generic macros and types may be defined here. The arch-specific
@@ -16,7 +17,11 @@
  * the layout of sys_stat_struct must not be defined here.
  */
 
-/* stat flags (WARNING, octal here) */
+/* stat flags (WARNING, octal here). We need to check for an existing
+ * definition because linux/stat.h may omit to define those if it finds
+ * that any glibc header was already included.
+ */
+#if !defined(S_IFMT)
 #define S_IFDIR        0040000
 #define S_IFCHR        0020000
 #define S_IFBLK        0060000
@@ -34,6 +39,22 @@
 #define S_ISLNK(mode)  (((mode) & S_IFMT) == S_IFLNK)
 #define S_ISSOCK(mode) (((mode) & S_IFMT) == S_IFSOCK)
 
+#define S_IRWXU 00700
+#define S_IRUSR 00400
+#define S_IWUSR 00200
+#define S_IXUSR 00100
+
+#define S_IRWXG 00070
+#define S_IRGRP 00040
+#define S_IWGRP 00020
+#define S_IXGRP 00010
+
+#define S_IRWXO 00007
+#define S_IROTH 00004
+#define S_IWOTH 00002
+#define S_IXOTH 00001
+#endif
+
 /* dirent types */
 #define DT_UNKNOWN     0x0
 #define DT_FIFO        0x1
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/5] tools/nolibc: Include linux/fcntl.h and remove duplicate code
  2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
                   ` (2 preceding siblings ...)
  2023-03-04 14:28 ` [PATCH 3/5] tools/nolibc: check for S_I* macros before defining them Willy Tarreau
@ 2023-03-04 14:28 ` Willy Tarreau
  2023-03-04 14:28 ` [PATCH 5/5] tools/nolibc: Add statx() and make stat() rely on statx() if necessary Willy Tarreau
  4 siblings, 0 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

From: Feiyang Chen <chenfeiyang@loongson.cn>

Include linux/fcntl.h for O_* and AT_*. asm/fcntl.h is included
by linux/fcntl.h, so it can be safely removed.

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/sys.h   | 2 +-
 tools/include/nolibc/types.h | 5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 115579e7f1db..41cad6d6137e 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -11,7 +11,6 @@
 #include "std.h"
 
 /* system includes */
-#include <asm/fcntl.h>   // for O_*
 #include <asm/unistd.h>
 #include <asm/signal.h>  // for SIGCHLD
 #include <asm/ioctls.h>
@@ -20,6 +19,7 @@
 #include <linux/loop.h>
 #include <linux/time.h>
 #include <linux/auxvec.h>
+#include <linux/fcntl.h> // for O_* and AT_*
 
 #include "arch.h"
 #include "errno.h"
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 47a0997d2d74..10823e5ac44b 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -81,11 +81,6 @@
 #define MAXPATHLEN     (PATH_MAX)
 #endif
 
-/* Special FD used by all the *at functions */
-#ifndef AT_FDCWD
-#define AT_FDCWD       (-100)
-#endif
-
 /* whence values for lseek() */
 #define SEEK_SET       0
 #define SEEK_CUR       1
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 5/5] tools/nolibc: Add statx() and make stat() rely on statx() if necessary
  2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
                   ` (3 preceding siblings ...)
  2023-03-04 14:28 ` [PATCH 4/5] tools/nolibc: Include linux/fcntl.h and remove duplicate code Willy Tarreau
@ 2023-03-04 14:28 ` Willy Tarreau
  4 siblings, 0 replies; 6+ messages in thread
From: Willy Tarreau @ 2023-03-04 14:28 UTC (permalink / raw)
  To: paulmck; +Cc: chenhuacai, chenfeiyang, linux-kernel, Willy Tarreau

From: Feiyang Chen <chenfeiyang@loongson.cn>

LoongArch and RISC-V 32-bit only have statx(). ARC, Hexagon, Nios2 and
OpenRISC have statx() and stat64() but not stat() or newstat(). Add
statx() and make stat() rely on statx() if necessary to make them happy.
We may just use statx() for all architectures in the future.

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/sys.h | 56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 41cad6d6137e..5d624dc63a42 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -20,6 +20,7 @@
 #include <linux/time.h>
 #include <linux/auxvec.h>
 #include <linux/fcntl.h> // for O_* and AT_*
+#include <linux/stat.h>  // for statx()
 
 #include "arch.h"
 #include "errno.h"
@@ -1090,12 +1091,66 @@ pid_t setsid(void)
 	return ret;
 }
 
+#if defined(__NR_statx)
+/*
+ * int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf);
+ */
+
+static __attribute__((unused))
+int sys_statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf)
+{
+	return my_syscall5(__NR_statx, fd, path, flags, mask, buf);
+}
+
+static __attribute__((unused))
+int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf)
+{
+	int ret = sys_statx(fd, path, flags, mask, buf);
+
+	if (ret < 0) {
+		SET_ERRNO(-ret);
+		ret = -1;
+	}
+	return ret;
+}
+#endif
 
 /*
  * int stat(const char *path, struct stat *buf);
  * Warning: the struct stat's layout is arch-dependent.
  */
 
+#if defined(__NR_statx) && !defined(__NR_newfstatat) && !defined(__NR_stat)
+/*
+ * Maybe we can just use statx() when available for all architectures?
+ */
+static __attribute__((unused))
+int sys_stat(const char *path, struct stat *buf)
+{
+	struct statx statx;
+	long ret;
+
+	ret = sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx);
+	buf->st_dev     = ((statx.stx_dev_minor & 0xff)
+			  | (statx.stx_dev_major << 8)
+			  | ((statx.stx_dev_minor & ~0xff) << 12));
+	buf->st_ino     = statx.stx_ino;
+	buf->st_mode    = statx.stx_mode;
+	buf->st_nlink   = statx.stx_nlink;
+	buf->st_uid     = statx.stx_uid;
+	buf->st_gid     = statx.stx_gid;
+	buf->st_rdev    = ((statx.stx_rdev_minor & 0xff)
+			  | (statx.stx_rdev_major << 8)
+			  | ((statx.stx_rdev_minor & ~0xff) << 12));
+	buf->st_size    = statx.stx_size;
+	buf->st_blksize = statx.stx_blksize;
+	buf->st_blocks  = statx.stx_blocks;
+	buf->st_atime   = statx.stx_atime.tv_sec;
+	buf->st_mtime   = statx.stx_mtime.tv_sec;
+	buf->st_ctime   = statx.stx_ctime.tv_sec;
+	return ret;
+}
+#else
 static __attribute__((unused))
 int sys_stat(const char *path, struct stat *buf)
 {
@@ -1125,6 +1180,7 @@ int sys_stat(const char *path, struct stat *buf)
 	buf->st_ctime   = stat.st_ctime;
 	return ret;
 }
+#endif
 
 static __attribute__((unused))
 int stat(const char *path, struct stat *buf)
-- 
2.17.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-04 14:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-04 14:28 [PATCH 0/5] tools/nolibc: cleanups, statx(), getuid() Willy Tarreau
2023-03-04 14:28 ` [PATCH 1/5] tools/nolibc: add getuid() and geteuid() Willy Tarreau
2023-03-04 14:28 ` [PATCH 2/5] selftests/nolibc: skip the chroot_root and link_dir tests when not privileged Willy Tarreau
2023-03-04 14:28 ` [PATCH 3/5] tools/nolibc: check for S_I* macros before defining them Willy Tarreau
2023-03-04 14:28 ` [PATCH 4/5] tools/nolibc: Include linux/fcntl.h and remove duplicate code Willy Tarreau
2023-03-04 14:28 ` [PATCH 5/5] tools/nolibc: Add statx() and make stat() rely on statx() if necessary Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).