linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS
@ 2025-06-10 12:29 Mark Brown
  2025-06-10 12:29 ` [PATCH v2 1/4] tools/nolibc: Replace ifdef with if defined() in sys.h Mark Brown
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Mark Brown @ 2025-06-10 12:29 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Catalin Marinas, Will Deacon, Shuah Khan
  Cc: linux-kernel, linux-arm-kernel, linux-kselftest, Mark Brown

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@kernel.org

---
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,
-- 
Mark Brown <broonie@kernel.org>


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

* [PATCH v2 1/4] tools/nolibc: Replace ifdef with if defined() in sys.h
  2025-06-10 12:29 [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS Mark Brown
@ 2025-06-10 12:29 ` Mark Brown
  2025-06-10 12:29 ` [PATCH v2 2/4] tools/nolibc: Provide vfork() Mark Brown
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2025-06-10 12:29 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Catalin Marinas, Will Deacon, Shuah Khan
  Cc: linux-kernel, linux-arm-kernel, linux-kselftest, Mark Brown

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);

-- 
2.39.5


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

* [PATCH v2 2/4] tools/nolibc: Provide vfork()
  2025-06-10 12:29 [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS Mark Brown
  2025-06-10 12:29 ` [PATCH v2 1/4] tools/nolibc: Replace ifdef with if defined() in sys.h Mark Brown
@ 2025-06-10 12:29 ` Mark Brown
  2025-06-10 16:42   ` Thomas Weißschuh
  2025-06-10 12:29 ` [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS Mark Brown
  2025-06-10 12:29 ` [PATCH v2 4/4] selftests/nolibc: Add coverage of vfork() Mark Brown
  3 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2025-06-10 12:29 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Catalin Marinas, Will Deacon, Shuah Khan
  Cc: linux-kernel, linux-arm-kernel, linux-kselftest, Mark Brown

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);

-- 
2.39.5


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

* [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS
  2025-06-10 12:29 [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS Mark Brown
  2025-06-10 12:29 ` [PATCH v2 1/4] tools/nolibc: Replace ifdef with if defined() in sys.h Mark Brown
  2025-06-10 12:29 ` [PATCH v2 2/4] tools/nolibc: Provide vfork() Mark Brown
@ 2025-06-10 12:29 ` Mark Brown
  2025-07-03 10:39   ` Catalin Marinas
  2025-06-10 12:29 ` [PATCH v2 4/4] selftests/nolibc: Add coverage of vfork() Mark Brown
  3 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2025-06-10 12:29 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Catalin Marinas, Will Deacon, Shuah Khan
  Cc: linux-kernel, linux-arm-kernel, linux-kselftest, Mark Brown

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)

-- 
2.39.5


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

* [PATCH v2 4/4] selftests/nolibc: Add coverage of vfork()
  2025-06-10 12:29 [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS Mark Brown
                   ` (2 preceding siblings ...)
  2025-06-10 12:29 ` [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS Mark Brown
@ 2025-06-10 12:29 ` Mark Brown
  3 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2025-06-10 12:29 UTC (permalink / raw)
  To: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Catalin Marinas, Will Deacon, Shuah Khan
  Cc: linux-kernel, linux-arm-kernel, linux-kselftest, Mark Brown

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;

-- 
2.39.5


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

* Re: [PATCH v2 2/4] tools/nolibc: Provide vfork()
  2025-06-10 12:29 ` [PATCH v2 2/4] tools/nolibc: Provide vfork() Mark Brown
@ 2025-06-10 16:42   ` Thomas Weißschuh
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Weißschuh @ 2025-06-10 16:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Willy Tarreau, Christian Brauner, Catalin Marinas, Will Deacon,
	Shuah Khan, linux-kernel, linux-arm-kernel, linux-kselftest

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
> 

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

* Re: [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS
  2025-06-10 12:29 ` [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS Mark Brown
@ 2025-07-03 10:39   ` Catalin Marinas
  2025-07-03 11:19     ` Mark Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2025-07-03 10:39 UTC (permalink / raw)
  To: Mark Brown
  Cc: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Will Deacon, Shuah Khan, linux-kernel, linux-arm-kernel,
	linux-kselftest

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.

-- 
Catalin

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

* Re: [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS
  2025-07-03 10:39   ` Catalin Marinas
@ 2025-07-03 11:19     ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2025-07-03 11:19 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Willy Tarreau, Thomas Weißschuh, Christian Brauner,
	Will Deacon, Shuah Khan, linux-kernel, linux-arm-kernel,
	linux-kselftest

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

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2025-07-03 11:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-10 12:29 [PATCH v2 0/4] kselftest/arm64: Add coverage for the interaction of vfork() and GCS Mark Brown
2025-06-10 12:29 ` [PATCH v2 1/4] tools/nolibc: Replace ifdef with if defined() in sys.h Mark Brown
2025-06-10 12:29 ` [PATCH v2 2/4] tools/nolibc: Provide vfork() Mark Brown
2025-06-10 16:42   ` Thomas Weißschuh
2025-06-10 12:29 ` [PATCH v2 3/4] kselftest/arm64: Add a test for vfork() with GCS Mark Brown
2025-07-03 10:39   ` Catalin Marinas
2025-07-03 11:19     ` Mark Brown
2025-06-10 12:29 ` [PATCH v2 4/4] selftests/nolibc: Add coverage of vfork() Mark Brown

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).