I had cause to look at the vfork() support for GCS and realised that we don't have any direct test coverage, this series does so by adding vfork() to nolibc and then using that in basic-gcs to provide some simple vfork() coverage.
Signed-off-by: Mark Brown broonie@kernel.org --- Changes in v2: - Add replacement of ifdef with if defined() in nolibc since the code doesn't reflect the coding style. - Remove check for arch specific vfork(). - Link to v1: https://lore.kernel.org/r/20250609-arm64-gcs-vfork-exit-v1-0-baad0f085747@ke...
--- Mark Brown (4): tools/nolibc: Replace ifdef with if defined() in sys.h tools/nolibc: Provide vfork() kselftest/arm64: Add a test for vfork() with GCS selftests/nolibc: Add coverage of vfork()
tools/include/nolibc/sys.h | 57 +++++++++++++++++------- tools/testing/selftests/arm64/gcs/basic-gcs.c | 63 +++++++++++++++++++++++++++ tools/testing/selftests/nolibc/nolibc-test.c | 23 ++++++++-- 3 files changed, 124 insertions(+), 19 deletions(-) --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250528-arm64-gcs-vfork-exit-4a7daf7652ee
Best regards,
Thomas has requested that if defined() be used in place of ifdef but currently ifdef is used consistently in sys.h. Update all the instances of ifdef to if defined().
Suggested-by: Thomas Weißschuh linux@weissschuh.net Signed-off-by: Mark Brown broonie@kernel.org --- tools/include/nolibc/sys.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 9556c69a6ae1..aabac97a7fb0 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -139,7 +139,7 @@ int chdir(const char *path) static __attribute__((unused)) int sys_chmod(const char *path, mode_t mode) { -#ifdef __NR_fchmodat +#if defined(__NR_fchmodat) return my_syscall4(__NR_fchmodat, AT_FDCWD, path, mode, 0); #elif defined(__NR_chmod) return my_syscall2(__NR_chmod, path, mode); @@ -162,7 +162,7 @@ int chmod(const char *path, mode_t mode) static __attribute__((unused)) int sys_chown(const char *path, uid_t owner, gid_t group) { -#ifdef __NR_fchownat +#if defined(__NR_fchownat) return my_syscall5(__NR_fchownat, AT_FDCWD, path, owner, group, 0); #elif defined(__NR_chown) return my_syscall3(__NR_chown, path, owner, group); @@ -236,7 +236,7 @@ int dup(int fd) static __attribute__((unused)) int sys_dup2(int old, int new) { -#ifdef __NR_dup3 +#if defined(__NR_dup3) return my_syscall3(__NR_dup3, old, new, 0); #elif defined(__NR_dup2) return my_syscall2(__NR_dup2, old, new); @@ -256,7 +256,7 @@ int dup2(int old, int new) * int dup3(int old, int new, int flags); */
-#ifdef __NR_dup3 +#if defined(__NR_dup3) static __attribute__((unused)) int sys_dup3(int old, int new, int flags) { @@ -320,7 +320,7 @@ void exit(int status) static __attribute__((unused)) pid_t sys_fork(void) { -#ifdef __NR_clone +#if defined(__NR_clone) /* note: some archs only have clone() and not fork(). Different archs * have a different API, but most archs have the flags on first arg and * will not use the rest with no other flag. @@ -382,7 +382,7 @@ int getdents64(int fd, struct linux_dirent64 *dirp, int count) static __attribute__((unused)) uid_t sys_geteuid(void) { -#ifdef __NR_geteuid32 +#if defined(__NR_geteuid32) return my_syscall0(__NR_geteuid32); #else return my_syscall0(__NR_geteuid); @@ -500,7 +500,7 @@ int getpagesize(void) static __attribute__((unused)) uid_t sys_getuid(void) { -#ifdef __NR_getuid32 +#if defined(__NR_getuid32) return my_syscall0(__NR_getuid32); #else return my_syscall0(__NR_getuid); @@ -538,7 +538,7 @@ int kill(pid_t pid, int signal) static __attribute__((unused)) int sys_link(const char *old, const char *new) { -#ifdef __NR_linkat +#if defined(__NR_linkat) return my_syscall5(__NR_linkat, AT_FDCWD, old, AT_FDCWD, new, 0); #elif defined(__NR_link) return my_syscall2(__NR_link, old, new); @@ -561,7 +561,7 @@ int link(const char *old, const char *new) static __attribute__((unused)) off_t sys_lseek(int fd, off_t offset, int whence) { -#ifdef __NR_lseek +#if defined(__NR_lseek) return my_syscall3(__NR_lseek, fd, offset, whence); #else return __nolibc_enosys(__func__, fd, offset, whence); @@ -572,7 +572,7 @@ static __attribute__((unused)) int sys_llseek(int fd, unsigned long offset_high, unsigned long offset_low, __kernel_loff_t *result, int whence) { -#ifdef __NR_llseek +#if defined(__NR_llseek) return my_syscall5(__NR_llseek, fd, offset_high, offset_low, result, whence); #else return __nolibc_enosys(__func__, fd, offset_high, offset_low, result, whence); @@ -609,7 +609,7 @@ off_t lseek(int fd, off_t offset, int whence) static __attribute__((unused)) int sys_mkdir(const char *path, mode_t mode) { -#ifdef __NR_mkdirat +#if defined(__NR_mkdirat) return my_syscall3(__NR_mkdirat, AT_FDCWD, path, mode); #elif defined(__NR_mkdir) return my_syscall2(__NR_mkdir, path, mode); @@ -631,7 +631,7 @@ int mkdir(const char *path, mode_t mode) static __attribute__((unused)) int sys_rmdir(const char *path) { -#ifdef __NR_rmdir +#if defined(__NR_rmdir) return my_syscall1(__NR_rmdir, path); #elif defined(__NR_unlinkat) return my_syscall3(__NR_unlinkat, AT_FDCWD, path, AT_REMOVEDIR); @@ -654,7 +654,7 @@ int rmdir(const char *path) static __attribute__((unused)) long sys_mknod(const char *path, mode_t mode, dev_t dev) { -#ifdef __NR_mknodat +#if defined(__NR_mknodat) return my_syscall4(__NR_mknodat, AT_FDCWD, path, mode, dev); #elif defined(__NR_mknod) return my_syscall3(__NR_mknod, path, mode, dev); @@ -843,7 +843,7 @@ pid_t setsid(void) static __attribute__((unused)) int sys_symlink(const char *old, const char *new) { -#ifdef __NR_symlinkat +#if defined(__NR_symlinkat) return my_syscall3(__NR_symlinkat, old, AT_FDCWD, new); #elif defined(__NR_symlink) return my_syscall2(__NR_symlink, old, new); @@ -900,7 +900,7 @@ int umount2(const char *path, int flags) static __attribute__((unused)) int sys_unlink(const char *path) { -#ifdef __NR_unlinkat +#if defined(__NR_unlinkat) return my_syscall3(__NR_unlinkat, AT_FDCWD, path, 0); #elif defined(__NR_unlink) return my_syscall1(__NR_unlink, path);
To allow testing of vfork() support in the arm64 basic-gcs test provide an implementation for nolibc, using the vfork() syscall if one is available and otherwise clone3(). We implement in terms of clone3() since the order of the arguments for clone() varies between architectures.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/include/nolibc/sys.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index aabac97a7fb0..5932ae8828a1 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -22,6 +22,7 @@ #include <linux/time.h> #include <linux/auxvec.h> #include <linux/fcntl.h> /* for O_* and AT_* */ +#include <linux/sched.h> /* for clone_args */ #include <linux/stat.h> /* for statx() */
#include "errno.h" @@ -340,6 +341,32 @@ pid_t fork(void) return __sysret(sys_fork()); }
+static __attribute__((unused)) +pid_t sys_vfork(void) +{ +#if defined(__NR_vfork) + return my_syscall0(__NR_vfork); +#elif defined(__NR_clone3) + /* + * clone() could be used but has different argument orders per + * architecture. + */ + struct clone_args args = { + .flags = CLONE_VM | CLONE_VFORK, + .exit_signal = SIGCHLD, + }; + + return my_syscall2(__NR_clone3, &args, sizeof(args)); +#else + return __nolibc_enosys(__func__); +#endif +} + +static __attribute__((unused)) +pid_t vfork(void) +{ + return __sysret(sys_vfork()); +}
/* * int fsync(int fd);
On 2025-06-10 13:29:45+0100, Mark Brown wrote:
To allow testing of vfork() support in the arm64 basic-gcs test provide an implementation for nolibc, using the vfork() syscall if one is available and otherwise clone3(). We implement in terms of clone3() since the order of the arguments for clone() varies between architectures.
Signed-off-by: Mark Brown broonie@kernel.org
tools/include/nolibc/sys.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index aabac97a7fb0..5932ae8828a1 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -22,6 +22,7 @@ #include <linux/time.h> #include <linux/auxvec.h> #include <linux/fcntl.h> /* for O_* and AT_* */ +#include <linux/sched.h> /* for clone_args */ #include <linux/stat.h> /* for statx() */ #include "errno.h" @@ -340,6 +341,32 @@ pid_t fork(void) return __sysret(sys_fork()); } +static __attribute__((unused)) +pid_t sys_vfork(void) +{ +#if defined(__NR_vfork)
- return my_syscall0(__NR_vfork);
+#elif defined(__NR_clone3)
- /*
* clone() could be used but has different argument orders per* architecture.*/- struct clone_args args = {
.flags = CLONE_VM | CLONE_VFORK,.exit_signal = SIGCHLD,- };
- return my_syscall2(__NR_clone3, &args, sizeof(args));
+#else
- return __nolibc_enosys(__func__);
+#endif +}
It seems that on SPARC __NR_vfork has the same non-standard return value as __NR_fork. It therefore needs the same special handling in arch-sparc.h. Maybe with a helper to avoid the duplication.
If you want I can fix this up when applying. Otherwise the series looks good.
+static __attribute__((unused)) +pid_t vfork(void) +{
- return __sysret(sys_vfork());
+} /*
- int fsync(int fd);
-- 2.39.5
Ensure that we've got at least some coverage of the special cases around vfork() by adding a test case in basic-gcs doing the same thing as the plain fork() one - vfork(), do a few checks and then return to the parent.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/gcs/basic-gcs.c | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+)
diff --git a/tools/testing/selftests/arm64/gcs/basic-gcs.c b/tools/testing/selftests/arm64/gcs/basic-gcs.c index 3fb9742342a3..96ea51cf7163 100644 --- a/tools/testing/selftests/arm64/gcs/basic-gcs.c +++ b/tools/testing/selftests/arm64/gcs/basic-gcs.c @@ -298,6 +298,68 @@ static bool test_fork(void) return pass; }
+/* A vfork()ed process can run and exit */ +static bool test_vfork(void) +{ + unsigned long child_mode; + int ret, status; + pid_t pid; + bool pass = true; + + pid = vfork(); + if (pid == -1) { + ksft_print_msg("vfork() failed: %d\n", errno); + pass = false; + goto out; + } + if (pid == 0) { + /* In child, make sure we can call a function, read + * the GCS pointer and status and then exit */ + valid_gcs_function(); + get_gcspr(); + + ret = my_syscall5(__NR_prctl, PR_GET_SHADOW_STACK_STATUS, + &child_mode, 0, 0, 0); + if (ret == 0 && !(child_mode & PR_SHADOW_STACK_ENABLE)) { + ksft_print_msg("GCS not enabled in child\n"); + ret = -EINVAL; + } + + exit(ret); + } + + /* + * In parent, check we can still do function calls then block + * for the child. + */ + valid_gcs_function(); + + ksft_print_msg("Waiting for child %d\n", pid); + + ret = waitpid(pid, &status, 0); + if (ret == -1) { + ksft_print_msg("Failed to wait for child: %d\n", + errno); + return false; + } + + if (!WIFEXITED(status)) { + ksft_print_msg("Child exited due to signal %d\n", + WTERMSIG(status)); + pass = false; + } else { + if (WEXITSTATUS(status)) { + ksft_print_msg("Child exited with status %d\n", + WEXITSTATUS(status)); + pass = false; + } + } + +out: + + return pass; +} + typedef bool (*gcs_test)(void);
static struct { @@ -314,6 +376,7 @@ static struct { { "enable_invalid", enable_invalid, true }, { "map_guarded_stack", map_guarded_stack }, { "fork", test_fork }, + { "vfork", test_vfork }, };
int main(void)
On Tue, Jun 10, 2025 at 01:29:46PM +0100, Mark Brown wrote:
diff --git a/tools/testing/selftests/arm64/gcs/basic-gcs.c b/tools/testing/selftests/arm64/gcs/basic-gcs.c index 3fb9742342a3..96ea51cf7163 100644 --- a/tools/testing/selftests/arm64/gcs/basic-gcs.c +++ b/tools/testing/selftests/arm64/gcs/basic-gcs.c @@ -298,6 +298,68 @@ static bool test_fork(void) return pass; } +/* A vfork()ed process can run and exit */ +static bool test_vfork(void) +{
- unsigned long child_mode;
- int ret, status;
- pid_t pid;
- bool pass = true;
- pid = vfork();
- if (pid == -1) {
ksft_print_msg("vfork() failed: %d\n", errno);pass = false;goto out;- }
- if (pid == 0) {
/* In child, make sure we can call a function, read* the GCS pointer and status and then exit */
Nit: coding style for multi-line comment. I guess we follow the kernel style.
valid_gcs_function();get_gcspr();ret = my_syscall5(__NR_prctl, PR_GET_SHADOW_STACK_STATUS,&child_mode, 0, 0, 0);if (ret == 0 && !(child_mode & PR_SHADOW_STACK_ENABLE)) {ksft_print_msg("GCS not enabled in child\n");ret = -EINVAL;
Does it make sense in user-space to pass negative values to exit()? I thought it should be between 0 and 255.
}exit(ret);
Should this be _exit() instead? IIRC exit() does some clean-ups which are not safe in the vfork'ed child.
- }
- /*
* In parent, check we can still do function calls then block* for the child.*/
The comment "block for the child" doesn't make sense in this context. vfork() already blocks the parent until exec() or _exit(). But I can see why you wanted waitpid() to retrieve the return status.
- valid_gcs_function();
- ksft_print_msg("Waiting for child %d\n", pid);
- ret = waitpid(pid, &status, 0);
- if (ret == -1) {
ksft_print_msg("Failed to wait for child: %d\n",errno);return false;- }
- if (!WIFEXITED(status)) {
ksft_print_msg("Child exited due to signal %d\n",WTERMSIG(status));pass = false;- } else {
if (WEXITSTATUS(status)) {
Nit: } else if {
ksft_print_msg("Child exited with status %d\n",WEXITSTATUS(status));pass = false;}- }
+out:
- return pass;
+}
typedef bool (*gcs_test)(void); static struct { @@ -314,6 +376,7 @@ static struct { { "enable_invalid", enable_invalid, true }, { "map_guarded_stack", map_guarded_stack }, { "fork", test_fork },
- { "vfork", test_vfork },
}; int main(void)
Other than the above, feel free add
Reviewed-by: Catalin Marinas catalin.marinas@arm.com
Thomas, do you want to merge this via your tree? Thanks.
On Thu, Jul 03, 2025 at 11:39:14AM +0100, Catalin Marinas wrote:
On Tue, Jun 10, 2025 at 01:29:46PM +0100, Mark Brown wrote:
exit(ret);
Should this be _exit() instead? IIRC exit() does some clean-ups which are not safe in the vfork'ed child.
This test is written to nolibc so that we don't get any of that libc level stuff, but yeah it would be a bit more correct.
Generalise the existing fork() test to also cover the newly added vfork() implementation.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/nolibc/nolibc-test.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index dbe13000fb1a..d682434c6442 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -877,7 +877,12 @@ int test_file_stream(void) return 0; }
-int test_fork(void) +enum fork_type { + FORK_STANDARD, + FORK_VFORK, +}; + +int test_fork(enum fork_type type) { int status; pid_t pid; @@ -886,14 +891,23 @@ int test_fork(void) fflush(stdout); fflush(stderr);
- pid = fork(); + switch (type) { + case FORK_STANDARD: + pid = fork(); + break; + case FORK_VFORK: + pid = vfork(); + break; + default: + return 1; + }
switch (pid) { case -1: return 1;
case 0: - exit(123); + _exit(123);
default: pid = waitpid(pid, &status, 0); @@ -1330,7 +1344,7 @@ int run_syscall(int min, int max) CASE_TEST(dup3_m1); tmp = dup3(-1, 100, 0); EXPECT_SYSER(1, tmp, -1, EBADF); if (tmp != -1) close(tmp); break; CASE_TEST(execve_root); EXPECT_SYSER(1, execve("/", (char*[]){ [0] = "/", [1] = NULL }, NULL), -1, EACCES); break; CASE_TEST(file_stream); EXPECT_SYSZR(1, test_file_stream()); break; - CASE_TEST(fork); EXPECT_SYSZR(1, test_fork()); break; + CASE_TEST(fork); EXPECT_SYSZR(1, test_fork(FORK_STANDARD)); break; CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(directories); EXPECT_SYSZR(proc, test_dirent()); break; @@ -1374,6 +1388,7 @@ int run_syscall(int min, int max) CASE_TEST(uname_fault); EXPECT_SYSER(1, uname(NULL), -1, EFAULT); break; CASE_TEST(unlink_root); EXPECT_SYSER(1, unlink("/"), -1, EISDIR); break; CASE_TEST(unlink_blah); EXPECT_SYSER(1, unlink("/proc/self/blah"), -1, ENOENT); break; + CASE_TEST(vfork); EXPECT_SYSZR(1, test_fork(FORK_VFORK)); break; CASE_TEST(wait_child); EXPECT_SYSER(1, wait(&tmp), -1, ECHILD); break; CASE_TEST(waitpid_min); EXPECT_SYSER(1, waitpid(INT_MIN, &tmp, WNOHANG), -1, ESRCH); break; CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
linux-kselftest-mirror@lists.linaro.org
-
Catalin Marinas -
Mark Brown -
Thomas Weißschuh