lkml.org 
[lkml]   [2022]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2] lkdtm: Add CFI_BACKWARD to test ROP mitigations
From
On 08/12/2022 06:22, Kees Cook wrote:
> On Tue, Dec 06, 2022 at 06:28:53PM -0600, Daniel D�az wrote:
>> Hello!
>>
>> On Sat, 16 Apr 2022 at 00:30, Kees Cook <keescook@chromium.org> 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@linux.alibaba.com>
>>> Signed-off-by: Dan Li <ashimida@linux.alibaba.com>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Signed-off-by: Kees Cook <keescook@chromium.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
>>
>> We're seeing this problem with allmodconfig on arm64 and GCC 8 (this
>> one observed on 6.0.12-rc3):
>>
>> -----8<----------8<----------8<-----
>> make --silent --keep-going --jobs=8
>> O=/home/tuxbuild/.cache/tuxmake/builds/2/build
>> CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- ARCH=arm64
>> CROSS_COMPILE=aarch64-linux-gnu- 'CC=sccache aarch64-linux-gnu-gcc'
>> 'HOSTCC=sccache gcc'
>> /builds/linux/drivers/misc/lkdtm/cfi.c:67:1: error: pragma or
>> attribute 'target("branch-protection=none")' is not valid
>> {
>> ^
>
> Uuuh... how is CONFIG_ARM64_PTR_AUTH_KERNEL getting set if the compiler
> can't support the 'target("branch-protection=none")' attribute?
>

Older GCC versions supported the (now deprecated) -msign-return-address option
instead of the newer -mbranch-protection option, and the kernel checks for that
too when setting CONFIG_ARM64_PTR_AUTH_KERNEL. I guess the test has never
compiled with older GCC versions. The following patch should fix it.

-- 8< --

Subject: [PATCH] lkdtm: cfi: Make PAC test work with GCC 7 and 8

The CFI test uses the branch-protection=none compiler attribute to
disable PAC return address protection on a function. While newer GCC
versions support this attribute, older versions (GCC 7 and 8) instead
supported the sign-return-address=none attribute, leading to a build
failure when the test is built with older compilers. Fix it by checking
which attribute is supported and using the correct one.

Fixes: 2e53b877dc12 ("lkdtm: Add CFI_BACKWARD to test ROP mitigations")
Reported-by: Daniel Díaz <daniel.diaz@linaro.org>
Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
Link: https://lore.kernel.org/all/CAEUSe78kDPxQmQqCWW-_9LCgJDFhAeMoVBFnX9QLx18Z4uT4VQ@mail.gmail.com/
---
drivers/misc/lkdtm/cfi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
index 5245cf6013c9..d4bb8e31a2fe 100644
--- a/drivers/misc/lkdtm/cfi.c
+++ b/drivers/misc/lkdtm/cfi.c
@@ -54,7 +54,11 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
# ifdef CONFIG_ARM64_BTI_KERNEL
# define __no_pac "branch-protection=bti"
# else
-# define __no_pac "branch-protection=none"
+# ifdef CONFIG_CC_HAS_BRANCH_PROT_PAC_RET
+# define __no_pac "branch-protection=none"
+# else
+# define __no_pac "sign-return-address=none"
+# endif
# endif
# define __no_ret_protection __noscs __attribute__((__target__(__no_pac)))
#else
\
 
 \ /
  Last update: 2022-12-09 18:35    [W:0.071 / U:1.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site