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

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

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.