[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e1563d18-36d7-08b2-762c-def7021d1382@linux.alibaba.com>
Date: Sun, 17 Apr 2022 02:15:43 -0700
From: Dan Li <ashimida@...ux.alibaba.com>
To: Kees Cook <keescook@...omium.org>
Cc: Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] lkdtm: Add CFI_BACKWARD to test ROP mitigations
On 4/15/22 17:11, Kees Cook wrote:
> In order to test various backward-edge control flow integrity methods,
> add a test that manipulates the return address on the stack. Currently
> only arm64 Pointer Authentication and Shadow Call Stack is supported.
>
> $ echo CFI_BACKWARD | cat >/sys/kernel/debug/provoke-crash/DIRECT
>
> Under SCS, successful test of the mitigation is reported as:
>
> lkdtm: Performing direct entry CFI_BACKWARD
> lkdtm: Attempting unchecked stack return address redirection ...
> lkdtm: ok: redirected stack return address.
> lkdtm: Attempting checked stack return address redirection ...
> lkdtm: ok: control flow unchanged.
>
> Under PAC, successful test of the mitigation is reported by the PAC
> exception handler:
>
> lkdtm: Performing direct entry CFI_BACKWARD
> lkdtm: Attempting unchecked stack return address redirection ...
> lkdtm: ok: redirected stack return address.
> lkdtm: Attempting checked stack return address redirection ...
> Unable to handle kernel paging request at virtual address bfffffc0088d0514
> Mem abort info:
> ESR = 0x86000004
> EC = 0x21: IABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> [bfffffc0088d0514] address between user and kernel address ranges
> ...
>
> If the CONFIGs are missing (or the mitigation isn't working), failure
> is reported as:
>
> lkdtm: Performing direct entry CFI_BACKWARD
> lkdtm: Attempting unchecked stack return address redirection ...
> lkdtm: ok: redirected stack return address.
> lkdtm: Attempting checked stack return address redirection ...
> lkdtm: FAIL: stack return address was redirected!
> lkdtm: This is probably expected, since this kernel was built *without* CONFIG_ARM64_PTR_AUTH_KERNEL=y nor CONFIG_SHADOW_CALL_STACK=y
>
> Co-developed-by: Dan Li <ashimida@...ux.alibaba.com>
> Signed-off-by: Dan Li <ashimida@...ux.alibaba.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> v1: https://lore.kernel.org/lkml/20220413213917.711770-1-keescook@chromium.org
> v2:
> - add PAGE_OFFSET setting for PAC bits (Dan Li)
> ---
> drivers/misc/lkdtm/cfi.c | 134 ++++++++++++++++++++++++
> tools/testing/selftests/lkdtm/tests.txt | 1 +
> 2 files changed, 135 insertions(+)
>
> diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
> index e88f778be0d5..804965a480b7 100644
> --- a/drivers/misc/lkdtm/cfi.c
> +++ b/drivers/misc/lkdtm/cfi.c
> @@ -3,6 +3,7 @@
> * This is for all the tests relating directly to Control Flow Integrity.
> */
> #include "lkdtm.h"
> +#include <asm/page.h>
>
> static int called_count;
>
> @@ -42,8 +43,141 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
> pr_expected_config(CONFIG_CFI_CLANG);
> }
>
> +/*
> + * This can stay local to LKDTM, as there should not be a production reason
> + * to disable PAC && SCS.
> + */
> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
> +# ifdef CONFIG_ARM64_BTI_KERNEL
> +# define __no_pac "branch-protection=bti"
> +# else
> +# define __no_pac "branch-protection=none"
> +# endif
> +# define __no_ret_protection __noscs __attribute__((__target__(__no_pac)))
> +#else
> +# define __no_ret_protection __noscs
> +#endif
> +
> +#define no_pac_addr(addr) \
> + ((__force __typeof__(addr))((__force u64)(addr) | PAGE_OFFSET))
> +
> +/* The ultimate ROP gadget. */
> +static noinline __no_ret_protection
> +void set_return_addr_unchecked(unsigned long *expected, unsigned long *addr)
> +{
> + /* Use of volatile is to make sure final write isn't seen as a dead store. */
> + unsigned long * volatile *ret_addr = (unsigned long **)__builtin_frame_address(0) + 1;
> +
> + /* Make sure we've found the right place on the stack before writing it. */
> + if (no_pac_addr(*ret_addr) == expected)
> + *ret_addr = (addr);
> + else
> + /* Check architecture, stack layout, or compiler behavior... */
> + pr_warn("Eek: return address mismatch! %px != %px\n",
> + *ret_addr, addr);
> +}
> +
> +static noinline
> +void set_return_addr(unsigned long *expected, unsigned long *addr)
> +{
> + /* Use of volatile is to make sure final write isn't seen as a dead store. */
> + unsigned long * volatile *ret_addr = (unsigned long **)__builtin_frame_address(0) + 1;
> +
> + /* Make sure we've found the right place on the stack before writing it. */
> + if (no_pac_addr(*ret_addr) == expected)
> + *ret_addr = (addr);
> + else
> + /* Check architecture, stack layout, or compiler behavior... */
> + pr_warn("Eek: return address mismatch! %px != %px\n",
> + *ret_addr, addr);
> +}
> +
> +static volatile int force_check;
> +
> +static void lkdtm_CFI_BACKWARD(void)
> +{
> + /* Use calculated gotos to keep labels addressable. */
> + void *labels[] = {0, &&normal, &&redirected, &&check_normal, &&check_redirected};
> +
> + pr_info("Attempting unchecked stack return address redirection ...\n");
> +
> + /* Always false */
> + if (force_check) {
> + /*
> + * Prepare to call with NULLs to avoid parameters being treated as
> + * constants in -02.
> + */
> + set_return_addr_unchecked(NULL, NULL);
> + set_return_addr(NULL, NULL);
> + if (force_check)
> + goto *labels[1];
> + if (force_check)
> + goto *labels[2];
> + if (force_check)
> + goto *labels[3];
> + if (force_check)
> + goto *labels[4];
> + return;
> + }
> +
> + /*
> + * Use fallthrough switch case to keep basic block ordering between
> + * set_return_addr*() and the label after it.
> + */
> + switch (force_check) {
> + case 0:
> + set_return_addr_unchecked(&&normal, &&redirected);
> + fallthrough;
> + case 1:
> +normal:
> + /* Always true */
> + if (!force_check) {
> + pr_err("FAIL: stack return address manipulation failed!\n");
> + /* If we can't redirect "normally", we can't test mitigations. */
> + return;
> + }
> + break;
> + default:
> +redirected:
> + pr_info("ok: redirected stack return address.\n");
> + break;
> + }
> +
> + pr_info("Attempting checked stack return address redirection ...\n");
> +
> + switch (force_check) {
> + case 0:
> + set_return_addr(&&check_normal, &&check_redirected);
> + fallthrough;
> + case 1:
> +check_normal:
> + /* Always true */
> + if (!force_check) {
> + pr_info("ok: control flow unchanged.\n");
> + return;
> + }
> +
> +check_redirected:
> + pr_err("FAIL: stack return address was redirected!\n");
> + break;
> + }
> +
> + if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL)) {
> + pr_expected_config(CONFIG_ARM64_PTR_AUTH_KERNEL);
> + return;
> + }
> + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) {
> + pr_expected_config(CONFIG_SHADOW_CALL_STACK);
> + return;
> + }
> + pr_warn("This is probably expected, since this %s was built *without* %s=y nor %s=y\n",
> + lkdtm_kernel_info,
> + "CONFIG_ARM64_PTR_AUTH_KERNEL", "CONFIG_SHADOW_CALL_STACK");
> +}
> +
> static struct crashtype crashtypes[] = {
> CRASHTYPE(CFI_FORWARD_PROTO),
> + CRASHTYPE(CFI_BACKWARD),
> };
>
> struct crashtype_category cfi_crashtypes = {
> diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
> index 243c781f0780..9dace01dbf15 100644
> --- a/tools/testing/selftests/lkdtm/tests.txt
> +++ b/tools/testing/selftests/lkdtm/tests.txt
> @@ -74,6 +74,7 @@ USERCOPY_STACK_BEYOND
> USERCOPY_KERNEL
> STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
> CFI_FORWARD_PROTO
> +CFI_BACKWARD call trace:|ok: control flow unchanged
> FORTIFIED_STRSCPY
> FORTIFIED_OBJECT
> FORTIFIED_SUBOBJECT
Compiling with gcc/llvm 12 on aarch64 platform with scs/pac enabled
respectively, all four cases work fine for me :)
Powered by blists - more mailing lists