From 2a0338769f27a38b5dd60b6bef5b83003789439b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 26 Apr 2022 17:09:11 -0700 Subject: lkdtm: cfi: Fix type width for masking PAC bits The masking for PAC bits wasn't handling 32-bit architectures correctly. Replace the u64 cast with uintptr_t. Reported-by: kernel test robot Reported-by: Geert Uytterhoeven Link: https://lore.kernel.org/lkml/CAMuHMdVz-J-1ZQ08u0bsQihDkcRmEPrtX5B_oRJ+Ns5jrasnUw@mail.gmail.com Fixes: 2e53b877dc12 ("lkdtm: Add CFI_BACKWARD to test ROP mitigations") Cc: Greg Kroah-Hartman Cc: Arnd Bergmann Signed-off-by: Kees Cook --- drivers/misc/lkdtm/cfi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/misc') diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c index 804965a480b7..666a7f4bc137 100644 --- a/drivers/misc/lkdtm/cfi.c +++ b/drivers/misc/lkdtm/cfi.c @@ -59,7 +59,7 @@ static void lkdtm_CFI_FORWARD_PROTO(void) #endif #define no_pac_addr(addr) \ - ((__force __typeof__(addr))((__force u64)(addr) | PAGE_OFFSET)) + ((__force __typeof__(addr))((uintptr_t)(addr) | PAGE_OFFSET)) /* The ultimate ROP gadget. */ static noinline __no_ret_protection -- cgit v1.2.3 From d2b8060f165105a68748a6d98ed548ca112ce4d3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 11 May 2022 22:46:09 -0700 Subject: lkdtm/usercopy: Rename "heap" to "slab" To more clearly distinguish between the various heap types, rename the slab tests to "slab". Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Shuah Khan Cc: linux-kselftest@vger.kernel.org Signed-off-by: Kees Cook --- drivers/misc/lkdtm/usercopy.c | 30 +++++++++++++++--------------- tools/testing/selftests/lkdtm/tests.txt | 8 ++++---- 2 files changed, 19 insertions(+), 19 deletions(-) (limited to 'drivers/misc') diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c index 7852b9fc7c47..945806db2a13 100644 --- a/drivers/misc/lkdtm/usercopy.c +++ b/drivers/misc/lkdtm/usercopy.c @@ -130,7 +130,7 @@ free_user: * This checks for whole-object size validation with hardened usercopy, * with or without usercopy whitelisting. */ -static void do_usercopy_heap_size(bool to_user) +static void do_usercopy_slab_size(bool to_user) { unsigned long user_addr; unsigned char *one, *two; @@ -196,9 +196,9 @@ free_kernel: /* * This checks for the specific whitelist window within an object. If this - * test passes, then do_usercopy_heap_size() tests will pass too. + * test passes, then do_usercopy_slab_size() tests will pass too. */ -static void do_usercopy_heap_whitelist(bool to_user) +static void do_usercopy_slab_whitelist(bool to_user) { unsigned long user_alloc; unsigned char *buf = NULL; @@ -272,24 +272,24 @@ free_alloc: } /* Callable tests. */ -static void lkdtm_USERCOPY_HEAP_SIZE_TO(void) +static void lkdtm_USERCOPY_SLAB_SIZE_TO(void) { - do_usercopy_heap_size(true); + do_usercopy_slab_size(true); } -static void lkdtm_USERCOPY_HEAP_SIZE_FROM(void) +static void lkdtm_USERCOPY_SLAB_SIZE_FROM(void) { - do_usercopy_heap_size(false); + do_usercopy_slab_size(false); } -static void lkdtm_USERCOPY_HEAP_WHITELIST_TO(void) +static void lkdtm_USERCOPY_SLAB_WHITELIST_TO(void) { - do_usercopy_heap_whitelist(true); + do_usercopy_slab_whitelist(true); } -static void lkdtm_USERCOPY_HEAP_WHITELIST_FROM(void) +static void lkdtm_USERCOPY_SLAB_WHITELIST_FROM(void) { - do_usercopy_heap_whitelist(false); + do_usercopy_slab_whitelist(false); } static void lkdtm_USERCOPY_STACK_FRAME_TO(void) @@ -358,10 +358,10 @@ void __exit lkdtm_usercopy_exit(void) } static struct crashtype crashtypes[] = { - CRASHTYPE(USERCOPY_HEAP_SIZE_TO), - CRASHTYPE(USERCOPY_HEAP_SIZE_FROM), - CRASHTYPE(USERCOPY_HEAP_WHITELIST_TO), - CRASHTYPE(USERCOPY_HEAP_WHITELIST_FROM), + CRASHTYPE(USERCOPY_SLAB_SIZE_TO), + CRASHTYPE(USERCOPY_SLAB_SIZE_FROM), + CRASHTYPE(USERCOPY_SLAB_WHITELIST_TO), + CRASHTYPE(USERCOPY_SLAB_WHITELIST_FROM), CRASHTYPE(USERCOPY_STACK_FRAME_TO), CRASHTYPE(USERCOPY_STACK_FRAME_FROM), CRASHTYPE(USERCOPY_STACK_BEYOND), diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt index 9dace01dbf15..65e53eb0840b 100644 --- a/tools/testing/selftests/lkdtm/tests.txt +++ b/tools/testing/selftests/lkdtm/tests.txt @@ -64,10 +64,10 @@ REFCOUNT_DEC_AND_TEST_SATURATED Saturation detected: still saturated REFCOUNT_SUB_AND_TEST_SATURATED Saturation detected: still saturated #REFCOUNT_TIMING timing only #ATOMIC_TIMING timing only -USERCOPY_HEAP_SIZE_TO -USERCOPY_HEAP_SIZE_FROM -USERCOPY_HEAP_WHITELIST_TO -USERCOPY_HEAP_WHITELIST_FROM +USERCOPY_SLAB_SIZE_TO +USERCOPY_SLAB_SIZE_FROM +USERCOPY_SLAB_WHITELIST_TO +USERCOPY_SLAB_WHITELIST_FROM USERCOPY_STACK_FRAME_TO USERCOPY_STACK_FRAME_FROM USERCOPY_STACK_BEYOND -- cgit v1.2.3 From fc34eec6869a5882fd339f4e200fad21eb284093 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 12 May 2022 10:19:41 -0700 Subject: lkdtm/usercopy: Check vmalloc and >0-order folios Add coverage for the recently added usercopy checks for vmalloc and folios, via USERCOPY_VMALLOC and USERCOPY_FOLIO respectively. Cc: Greg Kroah-Hartman Cc: Matthew Wilcox (Oracle) Cc: Arnd Bergmann Signed-off-by: Kees Cook --- drivers/misc/lkdtm/usercopy.c | 83 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) (limited to 'drivers/misc') diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c index 945806db2a13..6215ec995cd3 100644 --- a/drivers/misc/lkdtm/usercopy.c +++ b/drivers/misc/lkdtm/usercopy.c @@ -5,6 +5,7 @@ */ #include "lkdtm.h" #include +#include #include #include #include @@ -341,6 +342,86 @@ free_user: vm_munmap(user_addr, PAGE_SIZE); } +/* + * This expects "kaddr" to point to a PAGE_SIZE allocation, which means + * a more complete test that would include copy_from_user() would risk + * memory corruption. Just test copy_to_user() here, as that exercises + * almost exactly the same code paths. + */ +static void do_usercopy_page_span(const char *name, void *kaddr) +{ + unsigned long uaddr; + + uaddr = vm_mmap(NULL, 0, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0); + if (uaddr >= TASK_SIZE) { + pr_warn("Failed to allocate user memory\n"); + return; + } + + /* Initialize contents. */ + memset(kaddr, 0xAA, PAGE_SIZE); + + /* Bump the kaddr forward to detect a page-spanning overflow. */ + kaddr += PAGE_SIZE / 2; + + pr_info("attempting good copy_to_user() from kernel %s: %px\n", + name, kaddr); + if (copy_to_user((void __user *)uaddr, kaddr, + unconst + (PAGE_SIZE / 2))) { + pr_err("copy_to_user() failed unexpectedly?!\n"); + goto free_user; + } + + pr_info("attempting bad copy_to_user() from kernel %s: %px\n", + name, kaddr); + if (copy_to_user((void __user *)uaddr, kaddr, unconst + PAGE_SIZE)) { + pr_warn("Good, copy_to_user() failed, but lacked Oops(?!)\n"); + goto free_user; + } + + pr_err("FAIL: bad copy_to_user() not detected!\n"); + pr_expected_config_param(CONFIG_HARDENED_USERCOPY, "hardened_usercopy"); + +free_user: + vm_munmap(uaddr, PAGE_SIZE); +} + +static void lkdtm_USERCOPY_VMALLOC(void) +{ + void *addr; + + addr = vmalloc(PAGE_SIZE); + if (!addr) { + pr_err("vmalloc() failed!?\n"); + return; + } + do_usercopy_page_span("vmalloc", addr); + vfree(addr); +} + +static void lkdtm_USERCOPY_FOLIO(void) +{ + struct folio *folio; + void *addr; + + /* + * FIXME: Folio checking currently misses 0-order allocations, so + * allocate and bump forward to the last page. + */ + folio = folio_alloc(GFP_KERNEL | __GFP_ZERO, 1); + if (!folio) { + pr_err("folio_alloc() failed!?\n"); + return; + } + addr = folio_address(folio); + if (addr) + do_usercopy_page_span("folio", addr + PAGE_SIZE); + else + pr_err("folio_address() failed?!\n"); + folio_put(folio); +} + void __init lkdtm_usercopy_init(void) { /* Prepare cache that lacks SLAB_USERCOPY flag. */ @@ -365,6 +446,8 @@ static struct crashtype crashtypes[] = { CRASHTYPE(USERCOPY_STACK_FRAME_TO), CRASHTYPE(USERCOPY_STACK_FRAME_FROM), CRASHTYPE(USERCOPY_STACK_BEYOND), + CRASHTYPE(USERCOPY_VMALLOC), + CRASHTYPE(USERCOPY_FOLIO), CRASHTYPE(USERCOPY_KERNEL), }; -- cgit v1.2.3 From f260fd59e3f387432bda51072fff4494fba10b91 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 17 May 2022 14:44:20 -0700 Subject: lkdtm/heap: Hide allocation size from -Warray-bounds With the kmalloc() size annotations, GCC is smart enough to realize that LKDTM is intentionally writing past the end of the buffer. This is on purpose, of course, so hide the buffer from the optimizer. Silences: ../drivers/misc/lkdtm/heap.c: In function 'lkdtm_SLAB_LINEAR_OVERFLOW': ../drivers/misc/lkdtm/heap.c:59:13: warning: array subscript 256 is outside array bounds of 'void[1020]' [-Warray-bounds] 59 | data[1024 / sizeof(u32)] = 0x12345678; | ~~~~^~~~~~~~~~~~~~~~~~~~ In file included from ../drivers/misc/lkdtm/heap.c:7: In function 'kmalloc', inlined from 'lkdtm_SLAB_LINEAR_OVERFLOW' at ../drivers/misc/lkdtm/heap.c:54:14: ../include/linux/slab.h:581:24: note: at offset 1024 into object of size 1020 allocated by 'kmem_cache_alloc_trace' 581 | return kmem_cache_alloc_trace( | ^~~~~~~~~~~~~~~~~~~~~~~ 582 | kmalloc_caches[kmalloc_type(flags)][index], | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 583 | flags, size); | ~~~~~~~~~~~~ Cc: Greg Kroah-Hartman Cc: Arnd Bergmann Signed-off-by: Kees Cook --- drivers/misc/lkdtm/heap.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/misc') diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c index 5d3b92cd23bd..62516078a619 100644 --- a/drivers/misc/lkdtm/heap.c +++ b/drivers/misc/lkdtm/heap.c @@ -56,6 +56,7 @@ static void lkdtm_SLAB_LINEAR_OVERFLOW(void) return; pr_info("Attempting slab linear overflow ...\n"); + OPTIMIZER_HIDE_VAR(data); data[1024 / sizeof(u32)] = 0x12345678; kfree(data); } -- cgit v1.2.3