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.