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 <[email protected]>
Thomas, do you want to merge this via your tree? Thanks.
--
Catalin
