Mark Brown broonie@kernel.org writes:
- /* Same thing via process_vm_readv() */
- local_iov.iov_base = &rval;
- local_iov.iov_len = sizeof(rval);
- remote_iov.iov_base = (void *)gcspr;
- remote_iov.iov_len = sizeof(rval);
- ret = process_vm_writev(child, &local_iov, 1, &remote_iov, 1, 0);
- if (ret == -1)
ksft_print_msg("process_vm_readv() failed: %s (%d)\n",strerror(errno), errno);
The comment and the error message say "process_vm_readv()", but the function actually called is process_vm_writev(). Is this intended?
Also, process_vm_writev() is failing when I run on my Arm FVP:
# # RUN global.ptrace_read_write ... # # Child: 1150 # # Child GCSPR 0xffffa210ffd8, flags 1, locked 0 # # process_vm_readv() failed: Bad address (14) # # libc-gcs.c:271:ptrace_read_write:Expected ret (-1) == sizeof(rval) (8) # # libc-gcs.c:272:ptrace_read_write:Expected val (281473401005692) == rval (281473402849248) # # libc-gcs.c:293:ptrace_read_write:Expected val (281473401005692) == ptrace(PTRACE_PEEKDATA, child, (void *)gcspr, NULL) (0) # # ptrace_read_write: Test failed at step #1 # # FAIL global.ptrace_read_write # not ok 4 global.ptrace_read_write
If I swap process_vm_readv() and process_vm_writev(), then the read succeeds but the write fails:
# RUN global.ptrace_read_write ... # Child: 1996 # Child GCSPR 0xffffa7fcffd8, flags 1, locked 0 # process_vm_writev() failed: Bad address (14) # libc-gcs.c:291:ptrace_read_write:Expected ret (-1) == sizeof(rval) (8) # libc-gcs.c:293:ptrace_read_write:Expected val (281473500358268) == ptrace(PTRACE_PEEKDATA, child, (void *)gcspr, NULL) (0) # ptrace_read_write: Test failed at step #1 # FAIL global.ptrace_read_write not ok 4 global.ptrace_read_write
+/* Put it all together, we can safely switch to and from the stack */ +TEST_F(map_gcs, stack_switch) +{
- size_t cap_index;
- cap_index = (variant->stack_size / sizeof(unsigned long));
- unsigned long *orig_gcspr_el0, *pivot_gcspr_el0;
- /* Skip over the stack terminator and point at the cap */
- switch (variant->flags & (SHADOW_STACK_SET_MARKER | SHADOW_STACK_SET_TOKEN)) {
- case SHADOW_STACK_SET_MARKER | SHADOW_STACK_SET_TOKEN:
cap_index -= 2;break;- case SHADOW_STACK_SET_TOKEN:
cap_index -= 1;break;- case SHADOW_STACK_SET_MARKER:
- case 0:
/* No cap, no test */return;- }
- pivot_gcspr_el0 = &self->stack[cap_index];
- /* Pivot to the new GCS */
- ksft_print_msg("Pivoting to %p from %p, target has value 0x%lx\n",
pivot_gcspr_el0, get_gcspr(),*pivot_gcspr_el0);- gcsss1(pivot_gcspr_el0);
- orig_gcspr_el0 = gcsss2();
- ksft_print_msg("Pivoted to %p from %p, target has value 0x%lx\n",
pivot_gcspr_el0, get_gcspr(),
Not sure about the intent here, but perhaps "get_gcspr()" here should be "orig_gcspr_el0" instead? Ditto in the equivalent place at the map_gcs.stack_overflow test below.
Also, it's strange that the tests defined after map_gcs.stack_overflow don't run when I execute this test program. I'm doing:
$ ./run_kselftest.sh -t arm64:libc-gcs
I.e., these tests aren't being run in my FVP:
+FIXTURE_VARIANT_ADD(map_invalid_gcs, too_small) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_1) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_2) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_3) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_4) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_5) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_6) +FIXTURE_VARIANT_ADD(map_invalid_gcs, unligned_7) +TEST_F(map_invalid_gcs, do_map) +FIXTURE_VARIANT_ADD(invalid_mprotect, exec) +FIXTURE_VARIANT_ADD(invalid_mprotect, bti) +FIXTURE_VARIANT_ADD(invalid_mprotect, exec_bti) +TEST_F(invalid_mprotect, do_map) +TEST_F(invalid_mprotect, do_map_read)
Finally, one last comment:
+int main(int argc, char **argv) +{
- unsigned long gcs_mode;
- int ret;
- if (!(getauxval(AT_HWCAP2) & HWCAP2_GCS))
ksft_exit_skip("SKIP GCS not supported\n");- /*
* Force shadow stacks on, our tests *should* be fine with or* without libc support and with or without this having ended* up tagged for GCS and enabled by the dynamic linker. We* can't use the libc prctl() function since we can't return* from enabling the stack. Also lock GCS if not already* locked so we can test behaviour when it's locked.
This is probably a leftover from a previous version: the test doesn't lock any GCS flag.
*/- ret = my_syscall2(__NR_prctl, PR_GET_SHADOW_STACK_STATUS, &gcs_mode);
- if (ret) {
ksft_print_msg("Failed to read GCS state: %d\n", ret);return EXIT_FAILURE;- }
- if (!(gcs_mode & PR_SHADOW_STACK_ENABLE)) {
gcs_mode = PR_SHADOW_STACK_ENABLE;ret = my_syscall2(__NR_prctl, PR_SET_SHADOW_STACK_STATUS,gcs_mode);if (ret) {ksft_print_msg("Failed to configure GCS: %d\n", ret);return EXIT_FAILURE;}- }
- /* Avoid returning in case libc doesn't understand GCS */
- exit(test_harness_run(argc, argv));
+}