| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <875y1089i4.fsf@linaro.org>
Date: Thu, 14 Dec 2023 23:50:11 -0300
From: Thiago Jung Bauermann <thiago.bauermann@...aro.org>
To: Mark Brown <broonie@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon
<will@...nel.org>, Jonathan Corbet <corbet@....net>, Andrew Morton
<akpm@...ux-foundation.org>, Marc Zyngier <maz@...nel.org>, Oliver Upton
<oliver.upton@...ux.dev>, James Morse <james.morse@....com>, Suzuki K
Poulose <suzuki.poulose@....com>, Arnd Bergmann <arnd@...db.de>, Oleg
Nesterov <oleg@...hat.com>, Eric Biederman <ebiederm@...ssion.com>, Kees
Cook <keescook@...omium.org>, Shuah Khan <shuah@...nel.org>, "Rick P.
Edgecombe" <rick.p.edgecombe@...el.com>, Deepak Gupta
<debug@...osinc.com>, Ard Biesheuvel <ardb@...nel.org>, Szabolcs Nagy
<Szabolcs.Nagy@....com>, "H.J. Lu" <hjl.tools@...il.com>, Paul Walmsley
<paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, Albert Ou
<aou@...s.berkeley.edu>, Florian Weimer <fweimer@...hat.com>, Christian
Brauner <brauner@...nel.org>, linux-arm-kernel@...ts.infradead.org,
linux-doc@...r.kernel.org, kvmarm@...ts.linux.dev,
linux-fsdevel@...r.kernel.org, linux-arch@...r.kernel.org,
linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v7 34/39] kselftest/arm64: Add a GCS test program built
with the system libc
Mark Brown <broonie@...nel.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));
> +}
--
Thiago
Powered by blists - more mailing lists