* [PATCH 01/12] lkdtm: add usercopy test for blocking kernel text
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 02/12] lkdtm: drop "alloc_size" parameter Kees Cook
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
The upcoming HARDENED_USERCOPY checks will also block access to the
kernel text, so provide a test for this as well.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index a595a6f2615a..23d222a6c77d 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -47,6 +47,7 @@
#include <linux/vmalloc.h>
#include <linux/mman.h>
#include <asm/cacheflush.h>
+#include <asm/sections.h>
#ifdef CONFIG_IDE
#include <linux/ide.h>
@@ -120,6 +121,7 @@ enum ctype {
CT_USERCOPY_STACK_FRAME_TO,
CT_USERCOPY_STACK_FRAME_FROM,
CT_USERCOPY_STACK_BEYOND,
+ CT_USERCOPY_KERNEL,
};
static char* cp_name[] = {
@@ -171,6 +173,7 @@ static char* cp_type[] = {
"USERCOPY_STACK_FRAME_TO",
"USERCOPY_STACK_FRAME_FROM",
"USERCOPY_STACK_BEYOND",
+ "USERCOPY_KERNEL",
};
static struct jprobe lkdtm;
@@ -495,6 +498,35 @@ free_user:
vm_munmap(user_addr, PAGE_SIZE);
}
+static void do_usercopy_kernel(void)
+{
+ unsigned long user_addr;
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+
+ pr_info("attempting good copy_to_user from kernel rodata\n");
+ if (copy_to_user((void __user *)user_addr, test_text,
+ sizeof(test_text))) {
+ pr_warn("copy_to_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_to_user from kernel text\n");
+ if (copy_to_user((void __user *)user_addr, _stext, PAGE_SIZE)) {
+ pr_warn("copy_to_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+
+free_user:
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+
static void do_usercopy_heap_size(bool to_user)
{
unsigned long user_addr;
@@ -957,6 +989,9 @@ static void lkdtm_do_action(enum ctype which)
case CT_USERCOPY_STACK_BEYOND:
do_usercopy_stack(true, false);
break;
+ case CT_USERCOPY_KERNEL:
+ do_usercopy_kernel();
+ break;
case CT_NONE:
default:
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 02/12] lkdtm: drop "alloc_size" parameter
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
2016-07-06 22:33 ` [PATCH 01/12] lkdtm: add usercopy test for blocking kernel text Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 03/12] lkdtm: split usercopy tests to separate file Kees Cook
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
There is no good reason to have the alloc_size parameter currently. The
compiler-tricking value used to exercise the stack can just use a stack
address instead. Similarly hard-code cache_size.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 23d222a6c77d..80e7a46db178 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -185,8 +185,6 @@ static char* cpoint_name;
static char* cpoint_type;
static int cpoint_count = DEFAULT_COUNT;
static int recur_count = REC_NUM_DEFAULT;
-static int alloc_size = 1024;
-static size_t cache_size;
static enum cname cpoint = CN_INVALID;
static enum ctype cptype = CT_NONE;
@@ -195,6 +193,8 @@ static DEFINE_SPINLOCK(count_lock);
static DEFINE_SPINLOCK(lock_me_up);
static u8 data_area[EXEC_SIZE];
+
+static size_t cache_size = 1024;
static struct kmem_cache *bad_cache;
static const unsigned char test_text[] = "This is a test.\n";
@@ -211,9 +211,6 @@ MODULE_PARM_DESC(cpoint_type, " Crash Point Type, action to be taken on "\
module_param(cpoint_count, int, 0644);
MODULE_PARM_DESC(cpoint_count, " Crash Point Count, number of times the "\
"crash point is to be hit to trigger action");
-module_param(alloc_size, int, 0644);
-MODULE_PARM_DESC(alloc_size, " Size of allocation for user copy tests "\
- "(from 1 to PAGE_SIZE)");
static unsigned int jp_do_irq(unsigned int irq)
{
@@ -442,7 +439,7 @@ static noinline void do_usercopy_stack(bool to_user, bool bad_frame)
/* This is a pointer to outside our current stack frame. */
if (bad_frame) {
- bad_stack = do_usercopy_stack_callee(alloc_size);
+ bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
} else {
/* Put start address just inside stack. */
bad_stack = task_stack_page(current) + THREAD_SIZE;
@@ -531,7 +528,7 @@ static void do_usercopy_heap_size(bool to_user)
{
unsigned long user_addr;
unsigned char *one, *two;
- size_t size = clamp_t(int, alloc_size, 1, PAGE_SIZE);
+ size_t size = 1024;
one = kmalloc(size, GFP_KERNEL);
two = kmalloc(size, GFP_KERNEL);
@@ -565,8 +562,7 @@ static void do_usercopy_heap_size(bool to_user)
}
} else {
pr_info("attempting good copy_from_user of correct size\n");
- if (copy_from_user(one, (void __user *)user_addr,
- size)) {
+ if (copy_from_user(one, (void __user *)user_addr, size)) {
pr_warn("copy_from_user failed unexpectedly?!\n");
goto free_user;
}
@@ -1285,7 +1281,6 @@ static int __init lkdtm_module_init(void)
ro_after_init |= 0xAA;
/* Prepare cache that lacks SLAB_USERCOPY flag. */
- cache_size = clamp_t(int, alloc_size, 1, PAGE_SIZE);
bad_cache = kmem_cache_create("lkdtm-no-usercopy", cache_size, 0,
0, NULL);
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/12] lkdtm: split usercopy tests to separate file
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
2016-07-06 22:33 ` [PATCH 01/12] lkdtm: add usercopy test for blocking kernel text Kees Cook
2016-07-06 22:33 ` [PATCH 02/12] lkdtm: drop "alloc_size" parameter Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 04/12] lkdtm: split memory permissions " Kees Cook
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This splits the USERCOPY_* tests into the new lkdtm_usercopy.c file to
help separate things better for readability.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/Makefile | 1 +
drivers/misc/lkdtm.h | 13 ++
drivers/misc/lkdtm_core.c | 280 ++-----------------------------------
drivers/misc/lkdtm_usercopy.c | 316 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 343 insertions(+), 267 deletions(-)
create mode 100644 drivers/misc/lkdtm_usercopy.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7d45ed4a1549..e6b2778731ff 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_PANEL) += panel.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o
+lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o
OBJCOPYFLAGS :=
OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index 9531fa3be4c3..ef290a2c8816 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -1,6 +1,19 @@
#ifndef __LKDTM_H
#define __LKDTM_H
+/* lkdtm_rodata.c */
void lkdtm_rodata_do_nothing(void);
+/* lkdtm_usercopy.c */
+void __init lkdtm_usercopy_init(void);
+void __exit lkdtm_usercopy_exit(void);
+void lkdtm_USERCOPY_HEAP_SIZE_TO(void);
+void lkdtm_USERCOPY_HEAP_SIZE_FROM(void);
+void lkdtm_USERCOPY_HEAP_FLAG_TO(void);
+void lkdtm_USERCOPY_HEAP_FLAG_FROM(void);
+void lkdtm_USERCOPY_STACK_FRAME_TO(void);
+void lkdtm_USERCOPY_STACK_FRAME_FROM(void);
+void lkdtm_USERCOPY_STACK_BEYOND(void);
+void lkdtm_USERCOPY_KERNEL(void);
+
#endif
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 80e7a46db178..74376920ed55 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -47,7 +47,6 @@
#include <linux/vmalloc.h>
#include <linux/mman.h>
#include <asm/cacheflush.h>
-#include <asm/sections.h>
#ifdef CONFIG_IDE
#include <linux/ide.h>
@@ -194,10 +193,6 @@ static DEFINE_SPINLOCK(lock_me_up);
static u8 data_area[EXEC_SIZE];
-static size_t cache_size = 1024;
-static struct kmem_cache *bad_cache;
-
-static const unsigned char test_text[] = "This is a test.\n";
static const unsigned long rodata = 0xAA55AA55;
static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
@@ -404,255 +399,6 @@ static void execute_user_location(void *dst)
func();
}
-/*
- * Instead of adding -Wno-return-local-addr, just pass the stack address
- * through a function to obfuscate it from the compiler.
- */
-static noinline unsigned char *trick_compiler(unsigned char *stack)
-{
- return stack + 0;
-}
-
-static noinline unsigned char *do_usercopy_stack_callee(int value)
-{
- unsigned char buf[32];
- int i;
-
- /* Exercise stack to avoid everything living in registers. */
- for (i = 0; i < sizeof(buf); i++) {
- buf[i] = value & 0xff;
- }
-
- return trick_compiler(buf);
-}
-
-static noinline void do_usercopy_stack(bool to_user, bool bad_frame)
-{
- unsigned long user_addr;
- unsigned char good_stack[32];
- unsigned char *bad_stack;
- int i;
-
- /* Exercise stack to avoid everything living in registers. */
- for (i = 0; i < sizeof(good_stack); i++)
- good_stack[i] = test_text[i % sizeof(test_text)];
-
- /* This is a pointer to outside our current stack frame. */
- if (bad_frame) {
- bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
- } else {
- /* Put start address just inside stack. */
- bad_stack = task_stack_page(current) + THREAD_SIZE;
- bad_stack -= sizeof(unsigned long);
- }
-
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- return;
- }
-
- if (to_user) {
- pr_info("attempting good copy_to_user of local stack\n");
- if (copy_to_user((void __user *)user_addr, good_stack,
- sizeof(good_stack))) {
- pr_warn("copy_to_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_to_user of distant stack\n");
- if (copy_to_user((void __user *)user_addr, bad_stack,
- sizeof(good_stack))) {
- pr_warn("copy_to_user failed, but lacked Oops\n");
- goto free_user;
- }
- } else {
- /*
- * There isn't a safe way to not be protected by usercopy
- * if we're going to write to another thread's stack.
- */
- if (!bad_frame)
- goto free_user;
-
- pr_info("attempting good copy_from_user of local stack\n");
- if (copy_from_user(good_stack, (void __user *)user_addr,
- sizeof(good_stack))) {
- pr_warn("copy_from_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_from_user of distant stack\n");
- if (copy_from_user(bad_stack, (void __user *)user_addr,
- sizeof(good_stack))) {
- pr_warn("copy_from_user failed, but lacked Oops\n");
- goto free_user;
- }
- }
-
-free_user:
- vm_munmap(user_addr, PAGE_SIZE);
-}
-
-static void do_usercopy_kernel(void)
-{
- unsigned long user_addr;
-
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- return;
- }
-
- pr_info("attempting good copy_to_user from kernel rodata\n");
- if (copy_to_user((void __user *)user_addr, test_text,
- sizeof(test_text))) {
- pr_warn("copy_to_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_to_user from kernel text\n");
- if (copy_to_user((void __user *)user_addr, _stext, PAGE_SIZE)) {
- pr_warn("copy_to_user failed, but lacked Oops\n");
- goto free_user;
- }
-
-free_user:
- vm_munmap(user_addr, PAGE_SIZE);
-}
-
-static void do_usercopy_heap_size(bool to_user)
-{
- unsigned long user_addr;
- unsigned char *one, *two;
- size_t size = 1024;
-
- one = kmalloc(size, GFP_KERNEL);
- two = kmalloc(size, GFP_KERNEL);
- if (!one || !two) {
- pr_warn("Failed to allocate kernel memory\n");
- goto free_kernel;
- }
-
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- goto free_kernel;
- }
-
- memset(one, 'A', size);
- memset(two, 'B', size);
-
- if (to_user) {
- pr_info("attempting good copy_to_user of correct size\n");
- if (copy_to_user((void __user *)user_addr, one, size)) {
- pr_warn("copy_to_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_to_user of too large size\n");
- if (copy_to_user((void __user *)user_addr, one, 2 * size)) {
- pr_warn("copy_to_user failed, but lacked Oops\n");
- goto free_user;
- }
- } else {
- pr_info("attempting good copy_from_user of correct size\n");
- if (copy_from_user(one, (void __user *)user_addr, size)) {
- pr_warn("copy_from_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_from_user of too large size\n");
- if (copy_from_user(one, (void __user *)user_addr, 2 * size)) {
- pr_warn("copy_from_user failed, but lacked Oops\n");
- goto free_user;
- }
- }
-
-free_user:
- vm_munmap(user_addr, PAGE_SIZE);
-free_kernel:
- kfree(one);
- kfree(two);
-}
-
-static void do_usercopy_heap_flag(bool to_user)
-{
- unsigned long user_addr;
- unsigned char *good_buf = NULL;
- unsigned char *bad_buf = NULL;
-
- /* Make sure cache was prepared. */
- if (!bad_cache) {
- pr_warn("Failed to allocate kernel cache\n");
- return;
- }
-
- /*
- * Allocate one buffer from each cache (kmalloc will have the
- * SLAB_USERCOPY flag already, but "bad_cache" won't).
- */
- good_buf = kmalloc(cache_size, GFP_KERNEL);
- bad_buf = kmem_cache_alloc(bad_cache, GFP_KERNEL);
- if (!good_buf || !bad_buf) {
- pr_warn("Failed to allocate buffers from caches\n");
- goto free_alloc;
- }
-
- /* Allocate user memory we'll poke at. */
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- goto free_alloc;
- }
-
- memset(good_buf, 'A', cache_size);
- memset(bad_buf, 'B', cache_size);
-
- if (to_user) {
- pr_info("attempting good copy_to_user with SLAB_USERCOPY\n");
- if (copy_to_user((void __user *)user_addr, good_buf,
- cache_size)) {
- pr_warn("copy_to_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_to_user w/o SLAB_USERCOPY\n");
- if (copy_to_user((void __user *)user_addr, bad_buf,
- cache_size)) {
- pr_warn("copy_to_user failed, but lacked Oops\n");
- goto free_user;
- }
- } else {
- pr_info("attempting good copy_from_user with SLAB_USERCOPY\n");
- if (copy_from_user(good_buf, (void __user *)user_addr,
- cache_size)) {
- pr_warn("copy_from_user failed unexpectedly?!\n");
- goto free_user;
- }
-
- pr_info("attempting bad copy_from_user w/o SLAB_USERCOPY\n");
- if (copy_from_user(bad_buf, (void __user *)user_addr,
- cache_size)) {
- pr_warn("copy_from_user failed, but lacked Oops\n");
- goto free_user;
- }
- }
-
-free_user:
- vm_munmap(user_addr, PAGE_SIZE);
-free_alloc:
- if (bad_buf)
- kmem_cache_free(bad_cache, bad_buf);
- kfree(good_buf);
-}
static void lkdtm_do_action(enum ctype which)
{
@@ -965,28 +711,28 @@ static void lkdtm_do_action(enum ctype which)
return;
}
case CT_USERCOPY_HEAP_SIZE_TO:
- do_usercopy_heap_size(true);
+ lkdtm_USERCOPY_HEAP_SIZE_TO();
break;
case CT_USERCOPY_HEAP_SIZE_FROM:
- do_usercopy_heap_size(false);
+ lkdtm_USERCOPY_HEAP_SIZE_FROM();
break;
case CT_USERCOPY_HEAP_FLAG_TO:
- do_usercopy_heap_flag(true);
+ lkdtm_USERCOPY_HEAP_FLAG_TO();
break;
case CT_USERCOPY_HEAP_FLAG_FROM:
- do_usercopy_heap_flag(false);
+ lkdtm_USERCOPY_HEAP_FLAG_FROM();
break;
case CT_USERCOPY_STACK_FRAME_TO:
- do_usercopy_stack(true, true);
+ lkdtm_USERCOPY_STACK_FRAME_TO();
break;
case CT_USERCOPY_STACK_FRAME_FROM:
- do_usercopy_stack(false, true);
+ lkdtm_USERCOPY_STACK_FRAME_FROM();
break;
case CT_USERCOPY_STACK_BEYOND:
- do_usercopy_stack(true, false);
+ lkdtm_USERCOPY_STACK_BEYOND();
break;
case CT_USERCOPY_KERNEL:
- do_usercopy_kernel();
+ lkdtm_USERCOPY_KERNEL();
break;
case CT_NONE:
default:
@@ -1277,13 +1023,12 @@ static int __init lkdtm_module_init(void)
int n_debugfs_entries = 1; /* Assume only the direct entry */
int i;
+ /* Handle test-specific initialization. */
+ lkdtm_usercopy_init();
+
/* Make sure we can write to __ro_after_init values during __init */
ro_after_init |= 0xAA;
- /* Prepare cache that lacks SLAB_USERCOPY flag. */
- bad_cache = kmem_cache_create("lkdtm-no-usercopy", cache_size, 0,
- 0, NULL);
-
/* Register debugfs interface */
lkdtm_debugfs_root = debugfs_create_dir("provoke-crash", NULL);
if (!lkdtm_debugfs_root) {
@@ -1335,7 +1080,8 @@ static void __exit lkdtm_module_exit(void)
{
debugfs_remove_recursive(lkdtm_debugfs_root);
- kmem_cache_destroy(bad_cache);
+ /* Handle test-specific clean-up. */
+ lkdtm_usercopy_exit();
unregister_jprobe(&lkdtm);
pr_info("Crash point unregistered\n");
diff --git a/drivers/misc/lkdtm_usercopy.c b/drivers/misc/lkdtm_usercopy.c
new file mode 100644
index 000000000000..42ec6aa0dfb8
--- /dev/null
+++ b/drivers/misc/lkdtm_usercopy.c
@@ -0,0 +1,316 @@
+/*
+ * This is for all the tests related to copy_to_user() and copy_from_user()
+ * hardening.
+ */
+#define pr_fmt(fmt) "lkdtm: " fmt
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/mman.h>
+#include <linux/uaccess.h>
+#include <asm/cacheflush.h>
+#include <asm/sections.h>
+
+static size_t cache_size = 1024;
+static struct kmem_cache *bad_cache;
+
+static const unsigned char test_text[] = "This is a test.\n";
+
+/*
+ * Instead of adding -Wno-return-local-addr, just pass the stack address
+ * through a function to obfuscate it from the compiler.
+ */
+static noinline unsigned char *trick_compiler(unsigned char *stack)
+{
+ return stack + 0;
+}
+
+static noinline unsigned char *do_usercopy_stack_callee(int value)
+{
+ unsigned char buf[32];
+ int i;
+
+ /* Exercise stack to avoid everything living in registers. */
+ for (i = 0; i < sizeof(buf); i++) {
+ buf[i] = value & 0xff;
+ }
+
+ return trick_compiler(buf);
+}
+
+static noinline void do_usercopy_stack(bool to_user, bool bad_frame)
+{
+ unsigned long user_addr;
+ unsigned char good_stack[32];
+ unsigned char *bad_stack;
+ int i;
+
+ /* Exercise stack to avoid everything living in registers. */
+ for (i = 0; i < sizeof(good_stack); i++)
+ good_stack[i] = test_text[i % sizeof(test_text)];
+
+ /* This is a pointer to outside our current stack frame. */
+ if (bad_frame) {
+ bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
+ } else {
+ /* Put start address just inside stack. */
+ bad_stack = task_stack_page(current) + THREAD_SIZE;
+ bad_stack -= sizeof(unsigned long);
+ }
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+
+ if (to_user) {
+ pr_info("attempting good copy_to_user of local stack\n");
+ if (copy_to_user((void __user *)user_addr, good_stack,
+ sizeof(good_stack))) {
+ pr_warn("copy_to_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_to_user of distant stack\n");
+ if (copy_to_user((void __user *)user_addr, bad_stack,
+ sizeof(good_stack))) {
+ pr_warn("copy_to_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ } else {
+ /*
+ * There isn't a safe way to not be protected by usercopy
+ * if we're going to write to another thread's stack.
+ */
+ if (!bad_frame)
+ goto free_user;
+
+ pr_info("attempting good copy_from_user of local stack\n");
+ if (copy_from_user(good_stack, (void __user *)user_addr,
+ sizeof(good_stack))) {
+ pr_warn("copy_from_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_from_user of distant stack\n");
+ if (copy_from_user(bad_stack, (void __user *)user_addr,
+ sizeof(good_stack))) {
+ pr_warn("copy_from_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ }
+
+free_user:
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+
+static void do_usercopy_heap_size(bool to_user)
+{
+ unsigned long user_addr;
+ unsigned char *one, *two;
+ const size_t size = 1024;
+
+ one = kmalloc(size, GFP_KERNEL);
+ two = kmalloc(size, GFP_KERNEL);
+ if (!one || !two) {
+ pr_warn("Failed to allocate kernel memory\n");
+ goto free_kernel;
+ }
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ goto free_kernel;
+ }
+
+ memset(one, 'A', size);
+ memset(two, 'B', size);
+
+ if (to_user) {
+ pr_info("attempting good copy_to_user of correct size\n");
+ if (copy_to_user((void __user *)user_addr, one, size)) {
+ pr_warn("copy_to_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_to_user of too large size\n");
+ if (copy_to_user((void __user *)user_addr, one, 2 * size)) {
+ pr_warn("copy_to_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ } else {
+ pr_info("attempting good copy_from_user of correct size\n");
+ if (copy_from_user(one, (void __user *)user_addr, size)) {
+ pr_warn("copy_from_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_from_user of too large size\n");
+ if (copy_from_user(one, (void __user *)user_addr, 2 * size)) {
+ pr_warn("copy_from_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ }
+
+free_user:
+ vm_munmap(user_addr, PAGE_SIZE);
+free_kernel:
+ kfree(one);
+ kfree(two);
+}
+
+static void do_usercopy_heap_flag(bool to_user)
+{
+ unsigned long user_addr;
+ unsigned char *good_buf = NULL;
+ unsigned char *bad_buf = NULL;
+
+ /* Make sure cache was prepared. */
+ if (!bad_cache) {
+ pr_warn("Failed to allocate kernel cache\n");
+ return;
+ }
+
+ /*
+ * Allocate one buffer from each cache (kmalloc will have the
+ * SLAB_USERCOPY flag already, but "bad_cache" won't).
+ */
+ good_buf = kmalloc(cache_size, GFP_KERNEL);
+ bad_buf = kmem_cache_alloc(bad_cache, GFP_KERNEL);
+ if (!good_buf || !bad_buf) {
+ pr_warn("Failed to allocate buffers from caches\n");
+ goto free_alloc;
+ }
+
+ /* Allocate user memory we'll poke at. */
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ goto free_alloc;
+ }
+
+ memset(good_buf, 'A', cache_size);
+ memset(bad_buf, 'B', cache_size);
+
+ if (to_user) {
+ pr_info("attempting good copy_to_user with SLAB_USERCOPY\n");
+ if (copy_to_user((void __user *)user_addr, good_buf,
+ cache_size)) {
+ pr_warn("copy_to_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_to_user w/o SLAB_USERCOPY\n");
+ if (copy_to_user((void __user *)user_addr, bad_buf,
+ cache_size)) {
+ pr_warn("copy_to_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ } else {
+ pr_info("attempting good copy_from_user with SLAB_USERCOPY\n");
+ if (copy_from_user(good_buf, (void __user *)user_addr,
+ cache_size)) {
+ pr_warn("copy_from_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_from_user w/o SLAB_USERCOPY\n");
+ if (copy_from_user(bad_buf, (void __user *)user_addr,
+ cache_size)) {
+ pr_warn("copy_from_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+ }
+
+free_user:
+ vm_munmap(user_addr, PAGE_SIZE);
+free_alloc:
+ if (bad_buf)
+ kmem_cache_free(bad_cache, bad_buf);
+ kfree(good_buf);
+}
+
+/* Callable tests. */
+void lkdtm_USERCOPY_HEAP_SIZE_TO(void)
+{
+ do_usercopy_heap_size(true);
+}
+
+void lkdtm_USERCOPY_HEAP_SIZE_FROM(void)
+{
+ do_usercopy_heap_size(false);
+}
+
+void lkdtm_USERCOPY_HEAP_FLAG_TO(void)
+{
+ do_usercopy_heap_flag(true);
+}
+
+void lkdtm_USERCOPY_HEAP_FLAG_FROM(void)
+{
+ do_usercopy_heap_flag(false);
+}
+
+void lkdtm_USERCOPY_STACK_FRAME_TO(void)
+{
+ do_usercopy_stack(true, true);
+}
+
+void lkdtm_USERCOPY_STACK_FRAME_FROM(void)
+{
+ do_usercopy_stack(false, true);
+}
+
+void lkdtm_USERCOPY_STACK_BEYOND(void)
+{
+ do_usercopy_stack(true, false);
+}
+
+void lkdtm_USERCOPY_KERNEL(void)
+{
+ unsigned long user_addr;
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+
+ pr_info("attempting good copy_to_user from kernel rodata\n");
+ if (copy_to_user((void __user *)user_addr, test_text,
+ sizeof(test_text))) {
+ pr_warn("copy_to_user failed unexpectedly?!\n");
+ goto free_user;
+ }
+
+ pr_info("attempting bad copy_to_user from kernel text\n");
+ if (copy_to_user((void __user *)user_addr, _stext, PAGE_SIZE)) {
+ pr_warn("copy_to_user failed, but lacked Oops\n");
+ goto free_user;
+ }
+
+free_user:
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+
+void __init lkdtm_usercopy_init(void)
+{
+ /* Prepare cache that lacks SLAB_USERCOPY flag. */
+ bad_cache = kmem_cache_create("lkdtm-no-usercopy", cache_size, 0,
+ 0, NULL);
+}
+
+void __exit lkdtm_usercopy_exit(void)
+{
+ kmem_cache_destroy(bad_cache);
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/12] lkdtm: split memory permissions tests to separate file
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (2 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 03/12] lkdtm: split usercopy tests to separate file Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 05/12] lkdtm: split heap corruption " Kees Cook
` (7 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This splits the EXEC_*, WRITE_* and related tests into the new lkdtm_perms.c
file to help separate things better for readability.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/Makefile | 1 +
drivers/misc/lkdtm.h | 14 ++++
drivers/misc/lkdtm_core.c | 174 +++++---------------------------------
drivers/misc/lkdtm_perms.c | 203 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 237 insertions(+), 155 deletions(-)
create mode 100644 drivers/misc/lkdtm_perms.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e6b2778731ff..9f6e95bc0635 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PANEL) += panel.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
+lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index ef290a2c8816..40f681cd6efe 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -1,6 +1,19 @@
#ifndef __LKDTM_H
#define __LKDTM_H
+/* lkdtm_perms.c */
+void __init lkdtm_perms_init(void);
+void lkdtm_WRITE_RO(void);
+void lkdtm_WRITE_RO_AFTER_INIT(void);
+void lkdtm_WRITE_KERN(void);
+void lkdtm_EXEC_DATA(void);
+void lkdtm_EXEC_STACK(void);
+void lkdtm_EXEC_KMALLOC(void);
+void lkdtm_EXEC_VMALLOC(void);
+void lkdtm_EXEC_RODATA(void);
+void lkdtm_EXEC_USERSPACE(void);
+void lkdtm_ACCESS_USERSPACE(void);
+
/* lkdtm_rodata.c */
void lkdtm_rodata_do_nothing(void);
@@ -16,4 +29,5 @@ void lkdtm_USERCOPY_STACK_FRAME_FROM(void);
void lkdtm_USERCOPY_STACK_BEYOND(void);
void lkdtm_USERCOPY_KERNEL(void);
+
#endif
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 74376920ed55..0b3e3770068a 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -44,9 +44,6 @@
#include <linux/slab.h>
#include <scsi/scsi_cmnd.h>
#include <linux/debugfs.h>
-#include <linux/vmalloc.h>
-#include <linux/mman.h>
-#include <asm/cacheflush.h>
#ifdef CONFIG_IDE
#include <linux/ide.h>
@@ -67,7 +64,6 @@
#define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
#define DEFAULT_COUNT 10
-#define EXEC_SIZE 64
enum cname {
CN_INVALID,
@@ -191,11 +187,6 @@ static int count = DEFAULT_COUNT;
static DEFINE_SPINLOCK(count_lock);
static DEFINE_SPINLOCK(lock_me_up);
-static u8 data_area[EXEC_SIZE];
-
-static const unsigned long rodata = 0xAA55AA55;
-static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
-
module_param(recur_count, int, 0644);
MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test");
module_param(cpoint_name, charp, 0444);
@@ -348,18 +339,6 @@ static int recursive_loop(int remaining)
return recursive_loop(remaining - 1);
}
-static void do_nothing(void)
-{
- return;
-}
-
-/* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
-{
- pr_info("do_overwritten wasn't overwritten!\n");
- return;
-}
-
static noinline void corrupt_stack(void)
{
/* Use default char array length that triggers stack protection. */
@@ -368,38 +347,6 @@ static noinline void corrupt_stack(void)
memset((void *)data, 0, 64);
}
-static noinline void execute_location(void *dst, bool write)
-{
- void (*func)(void) = dst;
-
- pr_info("attempting ok execution at %p\n", do_nothing);
- do_nothing();
-
- if (write) {
- memcpy(dst, do_nothing, EXEC_SIZE);
- flush_icache_range((unsigned long)dst,
- (unsigned long)dst + EXEC_SIZE);
- }
- pr_info("attempting bad execution at %p\n", func);
- func();
-}
-
-static void execute_user_location(void *dst)
-{
- /* Intentionally crossing kernel/user memory boundary. */
- void (*func)(void) = dst;
-
- pr_info("attempting ok execution at %p\n", do_nothing);
- do_nothing();
-
- if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
- return;
- flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
- pr_info("attempting bad execution at %p\n", func);
- func();
-}
-
-
static void lkdtm_do_action(enum ctype which)
{
switch (which) {
@@ -577,116 +524,35 @@ static void lkdtm_do_action(enum ctype which)
schedule();
break;
case CT_EXEC_DATA:
- execute_location(data_area, true);
+ lkdtm_EXEC_DATA();
break;
- case CT_EXEC_STACK: {
- u8 stack_area[EXEC_SIZE];
- execute_location(stack_area, true);
+ case CT_EXEC_STACK:
+ lkdtm_EXEC_STACK();
break;
- }
- case CT_EXEC_KMALLOC: {
- u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL);
- execute_location(kmalloc_area, true);
- kfree(kmalloc_area);
+ case CT_EXEC_KMALLOC:
+ lkdtm_EXEC_KMALLOC();
break;
- }
- case CT_EXEC_VMALLOC: {
- u32 *vmalloc_area = vmalloc(EXEC_SIZE);
- execute_location(vmalloc_area, true);
- vfree(vmalloc_area);
+ case CT_EXEC_VMALLOC:
+ lkdtm_EXEC_VMALLOC();
break;
- }
case CT_EXEC_RODATA:
- execute_location(lkdtm_rodata_do_nothing, false);
+ lkdtm_EXEC_RODATA();
break;
- case CT_EXEC_USERSPACE: {
- unsigned long user_addr;
-
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- return;
- }
- execute_user_location((void *)user_addr);
- vm_munmap(user_addr, PAGE_SIZE);
+ case CT_EXEC_USERSPACE:
+ lkdtm_EXEC_USERSPACE();
break;
- }
- case CT_ACCESS_USERSPACE: {
- unsigned long user_addr, tmp = 0;
- unsigned long *ptr;
-
- user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_ANONYMOUS | MAP_PRIVATE, 0);
- if (user_addr >= TASK_SIZE) {
- pr_warn("Failed to allocate user memory\n");
- return;
- }
-
- if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
- pr_warn("copy_to_user failed\n");
- vm_munmap(user_addr, PAGE_SIZE);
- return;
- }
-
- ptr = (unsigned long *)user_addr;
-
- pr_info("attempting bad read at %p\n", ptr);
- tmp = *ptr;
- tmp += 0xc0dec0de;
-
- pr_info("attempting bad write at %p\n", ptr);
- *ptr = tmp;
-
- vm_munmap(user_addr, PAGE_SIZE);
-
+ case CT_ACCESS_USERSPACE:
+ lkdtm_ACCESS_USERSPACE();
break;
- }
- case CT_WRITE_RO: {
- /* Explicitly cast away "const" for the test. */
- unsigned long *ptr = (unsigned long *)&rodata;
-
- pr_info("attempting bad rodata write at %p\n", ptr);
- *ptr ^= 0xabcd1234;
-
+ case CT_WRITE_RO:
+ lkdtm_WRITE_RO();
break;
- }
- case CT_WRITE_RO_AFTER_INIT: {
- unsigned long *ptr = &ro_after_init;
-
- /*
- * Verify we were written to during init. Since an Oops
- * is considered a "success", a failure is to just skip the
- * real test.
- */
- if ((*ptr & 0xAA) != 0xAA) {
- pr_info("%p was NOT written during init!?\n", ptr);
- break;
- }
-
- pr_info("attempting bad ro_after_init write at %p\n", ptr);
- *ptr ^= 0xabcd1234;
-
+ case CT_WRITE_RO_AFTER_INIT:
+ lkdtm_WRITE_RO_AFTER_INIT();
break;
- }
- case CT_WRITE_KERN: {
- size_t size;
- unsigned char *ptr;
-
- size = (unsigned long)do_overwritten -
- (unsigned long)do_nothing;
- ptr = (unsigned char *)do_overwritten;
-
- pr_info("attempting bad %zu byte write at %p\n", size, ptr);
- memcpy(ptr, (unsigned char *)do_nothing, size);
- flush_icache_range((unsigned long)ptr,
- (unsigned long)(ptr + size));
-
- do_overwritten();
+ case CT_WRITE_KERN:
+ lkdtm_WRITE_KERN();
break;
- }
case CT_ATOMIC_UNDERFLOW: {
atomic_t under = ATOMIC_INIT(INT_MIN);
@@ -1024,11 +890,9 @@ static int __init lkdtm_module_init(void)
int i;
/* Handle test-specific initialization. */
+ lkdtm_perms_init();
lkdtm_usercopy_init();
- /* Make sure we can write to __ro_after_init values during __init */
- ro_after_init |= 0xAA;
-
/* Register debugfs interface */
lkdtm_debugfs_root = debugfs_create_dir("provoke-crash", NULL);
if (!lkdtm_debugfs_root) {
diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
new file mode 100644
index 000000000000..8201006502e2
--- /dev/null
+++ b/drivers/misc/lkdtm_perms.c
@@ -0,0 +1,203 @@
+/*
+ * This is for all the tests related to validating kernel memory
+ * permissions: non-executable regions, non-writable regions, and
+ * even non-readable regions.
+ */
+#define pr_fmt(fmt) "lkdtm: " fmt
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/mman.h>
+#include <linux/uaccess.h>
+#include <asm/cacheflush.h>
+
+#include "lkdtm.h"
+
+/* Whether or not to fill the target memory area with do_nothing(). */
+#define CODE_WRITE true
+#define CODE_AS_IS false
+
+/* How many bytes to copy to be sure we've copied enough of do_nothing(). */
+#define EXEC_SIZE 64
+
+/* This is non-const, so it will end up in the .data section. */
+static u8 data_area[EXEC_SIZE];
+
+/* This is cost, so it will end up in the .rodata section. */
+static const unsigned long rodata = 0xAA55AA55;
+
+/* This is marked __ro_after_init, so it should ultimately be .rodata. */
+static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
+
+/*
+ * This just returns to the caller. It is designed to be copied into
+ * non-executable memory regions.
+ */
+static void do_nothing(void)
+{
+ return;
+}
+
+/* Must immediately follow do_nothing for size calculuations to work out. */
+static void do_overwritten(void)
+{
+ pr_info("do_overwritten wasn't overwritten!\n");
+ return;
+}
+
+static noinline void execute_location(void *dst, bool write)
+{
+ void (*func)(void) = dst;
+
+ pr_info("attempting ok execution at %p\n", do_nothing);
+ do_nothing();
+
+ if (write == CODE_WRITE) {
+ memcpy(dst, do_nothing, EXEC_SIZE);
+ flush_icache_range((unsigned long)dst,
+ (unsigned long)dst + EXEC_SIZE);
+ }
+ pr_info("attempting bad execution at %p\n", func);
+ func();
+}
+
+static void execute_user_location(void *dst)
+{
+ /* Intentionally crossing kernel/user memory boundary. */
+ void (*func)(void) = dst;
+
+ pr_info("attempting ok execution at %p\n", do_nothing);
+ do_nothing();
+
+ if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE))
+ return;
+ flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE);
+ pr_info("attempting bad execution at %p\n", func);
+ func();
+}
+
+void lkdtm_WRITE_RO(void)
+{
+ /* Explicitly cast away "const" for the test. */
+ unsigned long *ptr = (unsigned long *)&rodata;
+
+ pr_info("attempting bad rodata write at %p\n", ptr);
+ *ptr ^= 0xabcd1234;
+}
+
+void lkdtm_WRITE_RO_AFTER_INIT(void)
+{
+ unsigned long *ptr = &ro_after_init;
+
+ /*
+ * Verify we were written to during init. Since an Oops
+ * is considered a "success", a failure is to just skip the
+ * real test.
+ */
+ if ((*ptr & 0xAA) != 0xAA) {
+ pr_info("%p was NOT written during init!?\n", ptr);
+ return;
+ }
+
+ pr_info("attempting bad ro_after_init write at %p\n", ptr);
+ *ptr ^= 0xabcd1234;
+}
+
+void lkdtm_WRITE_KERN(void)
+{
+ size_t size;
+ unsigned char *ptr;
+
+ size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
+ ptr = (unsigned char *)do_overwritten;
+
+ pr_info("attempting bad %zu byte write at %p\n", size, ptr);
+ memcpy(ptr, (unsigned char *)do_nothing, size);
+ flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size));
+
+ do_overwritten();
+}
+
+void lkdtm_EXEC_DATA(void)
+{
+ execute_location(data_area, CODE_WRITE);
+}
+
+void lkdtm_EXEC_STACK(void)
+{
+ u8 stack_area[EXEC_SIZE];
+ execute_location(stack_area, CODE_WRITE);
+}
+
+void lkdtm_EXEC_KMALLOC(void)
+{
+ u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL);
+ execute_location(kmalloc_area, CODE_WRITE);
+ kfree(kmalloc_area);
+}
+
+void lkdtm_EXEC_VMALLOC(void)
+{
+ u32 *vmalloc_area = vmalloc(EXEC_SIZE);
+ execute_location(vmalloc_area, CODE_WRITE);
+ vfree(vmalloc_area);
+}
+
+void lkdtm_EXEC_RODATA(void)
+{
+ execute_location(lkdtm_rodata_do_nothing, CODE_AS_IS);
+}
+
+void lkdtm_EXEC_USERSPACE(void)
+{
+ unsigned long user_addr;
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+ execute_user_location((void *)user_addr);
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+
+void lkdtm_ACCESS_USERSPACE(void)
+{
+ unsigned long user_addr, tmp = 0;
+ unsigned long *ptr;
+
+ user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0);
+ if (user_addr >= TASK_SIZE) {
+ pr_warn("Failed to allocate user memory\n");
+ return;
+ }
+
+ if (copy_to_user((void __user *)user_addr, &tmp, sizeof(tmp))) {
+ pr_warn("copy_to_user failed\n");
+ vm_munmap(user_addr, PAGE_SIZE);
+ return;
+ }
+
+ ptr = (unsigned long *)user_addr;
+
+ pr_info("attempting bad read at %p\n", ptr);
+ tmp = *ptr;
+ tmp += 0xc0dec0de;
+
+ pr_info("attempting bad write at %p\n", ptr);
+ *ptr = tmp;
+
+ vm_munmap(user_addr, PAGE_SIZE);
+}
+
+void __init lkdtm_perms_init(void)
+{
+ /* Make sure we can write to __ro_after_init values during __init */
+ ro_after_init |= 0xAA;
+
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/12] lkdtm: split heap corruption tests to separate file
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (3 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 04/12] lkdtm: split memory permissions " Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 06/12] lkdtm: split remaining logic bug " Kees Cook
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This splits the *_AFTER_FREE and related tests into the new lkdtm_heap.c
file to help separate things better for readability.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/Makefile | 1 +
drivers/misc/lkdtm.h | 7 +++
drivers/misc/lkdtm_core.c | 124 ++++-----------------------------------
drivers/misc/lkdtm_heap.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 164 insertions(+), 114 deletions(-)
create mode 100644 drivers/misc/lkdtm_heap.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 9f6e95bc0635..d66f657cf9f2 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PANEL) += panel.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
+lkdtm-$(CONFIG_LKDTM) += lkdtm_heap.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index 40f681cd6efe..9397360a2b11 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -1,6 +1,13 @@
#ifndef __LKDTM_H
#define __LKDTM_H
+/* lkdtm_heap.c */
+void lkdtm_OVERWRITE_ALLOCATION(void);
+void lkdtm_WRITE_AFTER_FREE(void);
+void lkdtm_READ_AFTER_FREE(void);
+void lkdtm_WRITE_BUDDY_AFTER_FREE(void);
+void lkdtm_READ_BUDDY_AFTER_FREE(void);
+
/* lkdtm_perms.c */
void __init lkdtm_perms_init(void);
void lkdtm_WRITE_RO(void);
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 0b3e3770068a..1c5056c286c5 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -384,125 +384,21 @@ static void lkdtm_do_action(enum ctype which)
*p = val;
break;
}
- case CT_OVERWRITE_ALLOCATION: {
- size_t len = 1020;
- u32 *data = kmalloc(len, GFP_KERNEL);
-
- data[1024 / sizeof(u32)] = 0x12345678;
- kfree(data);
+ case CT_OVERWRITE_ALLOCATION:
+ lkdtm_OVERWRITE_ALLOCATION();
break;
- }
- case CT_WRITE_AFTER_FREE: {
- int *base, *again;
- size_t len = 1024;
- /*
- * The slub allocator uses the first word to store the free
- * pointer in some configurations. Use the middle of the
- * allocation to avoid running into the freelist
- */
- size_t offset = (len / sizeof(*base)) / 2;
-
- base = kmalloc(len, GFP_KERNEL);
- pr_info("Allocated memory %p-%p\n", base, &base[offset * 2]);
- pr_info("Attempting bad write to freed memory at %p\n",
- &base[offset]);
- kfree(base);
- base[offset] = 0x0abcdef0;
- /* Attempt to notice the overwrite. */
- again = kmalloc(len, GFP_KERNEL);
- kfree(again);
- if (again != base)
- pr_info("Hmm, didn't get the same memory range.\n");
-
+ case CT_WRITE_AFTER_FREE:
+ lkdtm_WRITE_AFTER_FREE();
break;
- }
- case CT_READ_AFTER_FREE: {
- int *base, *val, saw;
- size_t len = 1024;
- /*
- * The slub allocator uses the first word to store the free
- * pointer in some configurations. Use the middle of the
- * allocation to avoid running into the freelist
- */
- size_t offset = (len / sizeof(*base)) / 2;
-
- base = kmalloc(len, GFP_KERNEL);
- if (!base)
- break;
-
- val = kmalloc(len, GFP_KERNEL);
- if (!val) {
- kfree(base);
- break;
- }
-
- *val = 0x12345678;
- base[offset] = *val;
- pr_info("Value in memory before free: %x\n", base[offset]);
-
- kfree(base);
-
- pr_info("Attempting bad read from freed memory\n");
- saw = base[offset];
- if (saw != *val) {
- /* Good! Poisoning happened, so declare a win. */
- pr_info("Memory correctly poisoned (%x)\n", saw);
- BUG();
- }
- pr_info("Memory was not poisoned\n");
-
- kfree(val);
+ case CT_READ_AFTER_FREE:
+ lkdtm_READ_AFTER_FREE();
break;
- }
- case CT_WRITE_BUDDY_AFTER_FREE: {
- unsigned long p = __get_free_page(GFP_KERNEL);
- if (!p)
- break;
- pr_info("Writing to the buddy page before free\n");
- memset((void *)p, 0x3, PAGE_SIZE);
- free_page(p);
- schedule();
- pr_info("Attempting bad write to the buddy page after free\n");
- memset((void *)p, 0x78, PAGE_SIZE);
- /* Attempt to notice the overwrite. */
- p = __get_free_page(GFP_KERNEL);
- free_page(p);
- schedule();
-
+ case CT_WRITE_BUDDY_AFTER_FREE:
+ lkdtm_WRITE_BUDDY_AFTER_FREE();
break;
- }
- case CT_READ_BUDDY_AFTER_FREE: {
- unsigned long p = __get_free_page(GFP_KERNEL);
- int saw, *val;
- int *base;
-
- if (!p)
- break;
-
- val = kmalloc(1024, GFP_KERNEL);
- if (!val) {
- free_page(p);
- break;
- }
-
- base = (int *)p;
-
- *val = 0x12345678;
- base[0] = *val;
- pr_info("Value in memory before free: %x\n", base[0]);
- free_page(p);
- pr_info("Attempting to read from freed memory\n");
- saw = base[0];
- if (saw != *val) {
- /* Good! Poisoning happened, so declare a win. */
- pr_info("Memory correctly poisoned (%x)\n", saw);
- BUG();
- }
- pr_info("Buddy page was not poisoned\n");
-
- kfree(val);
+ case CT_READ_BUDDY_AFTER_FREE:
+ lkdtm_READ_BUDDY_AFTER_FREE();
break;
- }
case CT_SOFTLOCKUP:
preempt_disable();
for (;;)
diff --git a/drivers/misc/lkdtm_heap.c b/drivers/misc/lkdtm_heap.c
new file mode 100644
index 000000000000..12f50e8dcbfe
--- /dev/null
+++ b/drivers/misc/lkdtm_heap.c
@@ -0,0 +1,146 @@
+/*
+ * This is for all the tests relating directly to heap memory, including
+ * page allocation and slab allocations.
+ */
+#define pr_fmt(fmt) "lkdtm: " fmt
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include "lkdtm.h"
+
+/*
+ * This tries to stay within the next largest power-of-2 kmalloc cache
+ * to avoid actually overwriting anything important if it's not detected
+ * correctly.
+ */
+void lkdtm_OVERWRITE_ALLOCATION(void)
+{
+ size_t len = 1020;
+ u32 *data = kmalloc(len, GFP_KERNEL);
+
+ data[1024 / sizeof(u32)] = 0x12345678;
+ kfree(data);
+}
+
+void lkdtm_WRITE_AFTER_FREE(void)
+{
+ int *base, *again;
+ size_t len = 1024;
+ /*
+ * The slub allocator uses the first word to store the free
+ * pointer in some configurations. Use the middle of the
+ * allocation to avoid running into the freelist
+ */
+ size_t offset = (len / sizeof(*base)) / 2;
+
+ base = kmalloc(len, GFP_KERNEL);
+ pr_info("Allocated memory %p-%p\n", base, &base[offset * 2]);
+ pr_info("Attempting bad write to freed memory at %p\n",
+ &base[offset]);
+ kfree(base);
+ base[offset] = 0x0abcdef0;
+ /* Attempt to notice the overwrite. */
+ again = kmalloc(len, GFP_KERNEL);
+ kfree(again);
+ if (again != base)
+ pr_info("Hmm, didn't get the same memory range.\n");
+}
+
+void lkdtm_READ_AFTER_FREE(void)
+{
+ int *base, *val, saw;
+ size_t len = 1024;
+ /*
+ * The slub allocator uses the first word to store the free
+ * pointer in some configurations. Use the middle of the
+ * allocation to avoid running into the freelist
+ */
+ size_t offset = (len / sizeof(*base)) / 2;
+
+ base = kmalloc(len, GFP_KERNEL);
+ if (!base) {
+ pr_info("Unable to allocate base memory.\n");
+ return;
+ }
+
+ val = kmalloc(len, GFP_KERNEL);
+ if (!val) {
+ pr_info("Unable to allocate val memory.\n");
+ kfree(base);
+ return;
+ }
+
+ *val = 0x12345678;
+ base[offset] = *val;
+ pr_info("Value in memory before free: %x\n", base[offset]);
+
+ kfree(base);
+
+ pr_info("Attempting bad read from freed memory\n");
+ saw = base[offset];
+ if (saw != *val) {
+ /* Good! Poisoning happened, so declare a win. */
+ pr_info("Memory correctly poisoned (%x)\n", saw);
+ BUG();
+ }
+ pr_info("Memory was not poisoned\n");
+
+ kfree(val);
+}
+
+void lkdtm_WRITE_BUDDY_AFTER_FREE(void)
+{
+ unsigned long p = __get_free_page(GFP_KERNEL);
+ if (!p) {
+ pr_info("Unable to allocate free page\n");
+ return;
+ }
+
+ pr_info("Writing to the buddy page before free\n");
+ memset((void *)p, 0x3, PAGE_SIZE);
+ free_page(p);
+ schedule();
+ pr_info("Attempting bad write to the buddy page after free\n");
+ memset((void *)p, 0x78, PAGE_SIZE);
+ /* Attempt to notice the overwrite. */
+ p = __get_free_page(GFP_KERNEL);
+ free_page(p);
+ schedule();
+}
+
+void lkdtm_READ_BUDDY_AFTER_FREE(void)
+{
+ unsigned long p = __get_free_page(GFP_KERNEL);
+ int saw, *val;
+ int *base;
+
+ if (!p) {
+ pr_info("Unable to allocate free page\n");
+ return;
+ }
+
+ val = kmalloc(1024, GFP_KERNEL);
+ if (!val) {
+ pr_info("Unable to allocate val memory.\n");
+ free_page(p);
+ return;
+ }
+
+ base = (int *)p;
+
+ *val = 0x12345678;
+ base[0] = *val;
+ pr_info("Value in memory before free: %x\n", base[0]);
+ free_page(p);
+ pr_info("Attempting to read from freed memory\n");
+ saw = base[0];
+ if (saw != *val) {
+ /* Good! Poisoning happened, so declare a win. */
+ pr_info("Memory correctly poisoned (%x)\n", saw);
+ BUG();
+ }
+ pr_info("Buddy page was not poisoned\n");
+
+ kfree(val);
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/12] lkdtm: split remaining logic bug tests to separate file
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (4 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 05/12] lkdtm: split heap corruption " Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 07/12] lkdtm: remove intentional off-by-one array access Kees Cook
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This splits all the remaining tests from lkdtm_core.c into the new
lkdtm_bugs.c file to help separate things better for readability.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/Makefile | 1 +
drivers/misc/lkdtm.h | 17 ++++++
drivers/misc/lkdtm_bugs.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/misc/lkdtm_core.c | 116 ++++++++---------------------------
4 files changed, 195 insertions(+), 91 deletions(-)
create mode 100644 drivers/misc/lkdtm_bugs.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index d66f657cf9f2..4387ccb79e64 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PANEL) += panel.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
+lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_heap.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index 9397360a2b11..d70a41741bb3 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -1,6 +1,23 @@
#ifndef __LKDTM_H
#define __LKDTM_H
+/* lkdtm_bugs.c */
+void __init lkdtm_bugs_init(int *recur_param);
+void lkdtm_PANIC(void);
+void lkdtm_BUG(void);
+void lkdtm_WARNING(void);
+void lkdtm_EXCEPTION(void);
+void lkdtm_LOOP(void);
+void lkdtm_OVERFLOW(void);
+void lkdtm_CORRUPT_STACK(void);
+void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void);
+void lkdtm_SOFTLOCKUP(void);
+void lkdtm_HARDLOCKUP(void);
+void lkdtm_SPINLOCKUP(void);
+void lkdtm_HUNG_TASK(void);
+void lkdtm_ATOMIC_UNDERFLOW(void);
+void lkdtm_ATOMIC_OVERFLOW(void);
+
/* lkdtm_heap.c */
void lkdtm_OVERWRITE_ALLOCATION(void);
void lkdtm_WRITE_AFTER_FREE(void);
diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c
new file mode 100644
index 000000000000..e87071f9c003
--- /dev/null
+++ b/drivers/misc/lkdtm_bugs.c
@@ -0,0 +1,152 @@
+/*
+ * This is for all the tests related to logic bugs (e.g. bad dereferences,
+ * bad alignment, bad loops, bad locking, bad scheduling, deep stacks, and
+ * lockups) along with other things that don't fit well into existing LKDTM
+ * test source files.
+ */
+#define pr_fmt(fmt) "lkdtm: " fmt
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+
+#include "lkdtm.h"
+
+/*
+ * Make sure our attempts to over run the kernel stack doesn't trigger
+ * a compiler warning when CONFIG_FRAME_WARN is set. Then make sure we
+ * recurse past the end of THREAD_SIZE by default.
+ */
+#if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
+#define REC_STACK_SIZE (CONFIG_FRAME_WARN / 2)
+#else
+#define REC_STACK_SIZE (THREAD_SIZE / 8)
+#endif
+#define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
+
+static int recur_count = REC_NUM_DEFAULT;
+
+static DEFINE_SPINLOCK(lock_me_up);
+
+static int recursive_loop(int remaining)
+{
+ char buf[REC_STACK_SIZE];
+
+ /* Make sure compiler does not optimize this away. */
+ memset(buf, (remaining & 0xff) | 0x1, REC_STACK_SIZE);
+ if (!remaining)
+ return 0;
+ else
+ return recursive_loop(remaining - 1);
+}
+
+/* If the depth is negative, use the default, otherwise keep parameter. */
+void __init lkdtm_bugs_init(int *recur_param)
+{
+ if (*recur_param < 0)
+ *recur_param = recur_count;
+ else
+ recur_count = *recur_param;
+}
+
+void lkdtm_PANIC(void)
+{
+ panic("dumptest");
+}
+
+void lkdtm_BUG(void)
+{
+ BUG();
+}
+
+void lkdtm_WARNING(void)
+{
+ WARN_ON(1);
+}
+
+void lkdtm_EXCEPTION(void)
+{
+ *((int *) 0) = 0;
+}
+
+void lkdtm_LOOP(void)
+{
+ for (;;)
+ ;
+}
+
+void lkdtm_OVERFLOW(void)
+{
+ (void) recursive_loop(recur_count);
+}
+
+noinline void lkdtm_CORRUPT_STACK(void)
+{
+ /* Use default char array length that triggers stack protection. */
+ char data[8];
+
+ memset((void *)data, 0, 64);
+}
+
+void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void)
+{
+ static u8 data[5] __attribute__((aligned(4))) = {1, 2, 3, 4, 5};
+ u32 *p;
+ u32 val = 0x12345678;
+
+ p = (u32 *)(data + 1);
+ if (*p == 0)
+ val = 0x87654321;
+ *p = val;
+}
+
+void lkdtm_SOFTLOCKUP(void)
+{
+ preempt_disable();
+ for (;;)
+ cpu_relax();
+}
+
+void lkdtm_HARDLOCKUP(void)
+{
+ local_irq_disable();
+ for (;;)
+ cpu_relax();
+}
+
+void lkdtm_SPINLOCKUP(void)
+{
+ /* Must be called twice to trigger. */
+ spin_lock(&lock_me_up);
+ /* Let sparse know we intended to exit holding the lock. */
+ __release(&lock_me_up);
+}
+
+void lkdtm_HUNG_TASK(void)
+{
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule();
+}
+
+void lkdtm_ATOMIC_UNDERFLOW(void)
+{
+ atomic_t under = ATOMIC_INIT(INT_MIN);
+
+ pr_info("attempting good atomic increment\n");
+ atomic_inc(&under);
+ atomic_dec(&under);
+
+ pr_info("attempting bad atomic underflow\n");
+ atomic_dec(&under);
+}
+
+void lkdtm_ATOMIC_OVERFLOW(void)
+{
+ atomic_t over = ATOMIC_INIT(INT_MAX);
+
+ pr_info("attempting good atomic decrement\n");
+ atomic_dec(&over);
+ atomic_inc(&over);
+
+ pr_info("attempting bad atomic overflow\n");
+ atomic_inc(&over);
+}
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 1c5056c286c5..e0f10131511f 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -51,20 +51,11 @@
#include "lkdtm.h"
-/*
- * Make sure our attempts to over run the kernel stack doesn't trigger
- * a compiler warning when CONFIG_FRAME_WARN is set. Then make sure we
- * recurse past the end of THREAD_SIZE by default.
- */
-#if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
-#define REC_STACK_SIZE (CONFIG_FRAME_WARN / 2)
-#else
-#define REC_STACK_SIZE (THREAD_SIZE / 8)
-#endif
-#define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
-
#define DEFAULT_COUNT 10
+static int count = DEFAULT_COUNT;
+static DEFINE_SPINLOCK(count_lock);
+
enum cname {
CN_INVALID,
CN_INT_HARDWARE_ENTRY,
@@ -179,13 +170,10 @@ static void lkdtm_handler(void);
static char* cpoint_name;
static char* cpoint_type;
static int cpoint_count = DEFAULT_COUNT;
-static int recur_count = REC_NUM_DEFAULT;
+static int recur_count = -1;
static enum cname cpoint = CN_INVALID;
static enum ctype cptype = CT_NONE;
-static int count = DEFAULT_COUNT;
-static DEFINE_SPINLOCK(count_lock);
-static DEFINE_SPINLOCK(lock_me_up);
module_param(recur_count, int, 0644);
MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test");
@@ -327,63 +315,33 @@ static int lkdtm_parse_commandline(void)
return -EINVAL;
}
-static int recursive_loop(int remaining)
-{
- char buf[REC_STACK_SIZE];
-
- /* Make sure compiler does not optimize this away. */
- memset(buf, (remaining & 0xff) | 0x1, REC_STACK_SIZE);
- if (!remaining)
- return 0;
- else
- return recursive_loop(remaining - 1);
-}
-
-static noinline void corrupt_stack(void)
-{
- /* Use default char array length that triggers stack protection. */
- char data[8];
-
- memset((void *)data, 0, 64);
-}
-
static void lkdtm_do_action(enum ctype which)
{
switch (which) {
case CT_PANIC:
- panic("dumptest");
+ lkdtm_PANIC();
break;
case CT_BUG:
- BUG();
+ lkdtm_BUG();
break;
case CT_WARNING:
- WARN_ON(1);
+ lkdtm_WARNING();
break;
case CT_EXCEPTION:
- *((int *) 0) = 0;
+ lkdtm_EXCEPTION();
break;
case CT_LOOP:
- for (;;)
- ;
+ lkdtm_LOOP();
break;
case CT_OVERFLOW:
- (void) recursive_loop(recur_count);
+ lkdtm_OVERFLOW();
break;
case CT_CORRUPT_STACK:
- corrupt_stack();
- break;
- case CT_UNALIGNED_LOAD_STORE_WRITE: {
- static u8 data[5] __attribute__((aligned(4))) = {1, 2,
- 3, 4, 5};
- u32 *p;
- u32 val = 0x12345678;
-
- p = (u32 *)(data + 1);
- if (*p == 0)
- val = 0x87654321;
- *p = val;
- break;
- }
+ lkdtm_CORRUPT_STACK();
+ break;
+ case CT_UNALIGNED_LOAD_STORE_WRITE:
+ lkdtm_UNALIGNED_LOAD_STORE_WRITE();
+ break;
case CT_OVERWRITE_ALLOCATION:
lkdtm_OVERWRITE_ALLOCATION();
break;
@@ -400,24 +358,16 @@ static void lkdtm_do_action(enum ctype which)
lkdtm_READ_BUDDY_AFTER_FREE();
break;
case CT_SOFTLOCKUP:
- preempt_disable();
- for (;;)
- cpu_relax();
+ lkdtm_SOFTLOCKUP();
break;
case CT_HARDLOCKUP:
- local_irq_disable();
- for (;;)
- cpu_relax();
+ lkdtm_HARDLOCKUP();
break;
case CT_SPINLOCKUP:
- /* Must be called twice to trigger. */
- spin_lock(&lock_me_up);
- /* Let sparse know we intended to exit holding the lock. */
- __release(&lock_me_up);
+ lkdtm_SPINLOCKUP();
break;
case CT_HUNG_TASK:
- set_current_state(TASK_UNINTERRUPTIBLE);
- schedule();
+ lkdtm_HUNG_TASK();
break;
case CT_EXEC_DATA:
lkdtm_EXEC_DATA();
@@ -449,29 +399,12 @@ static void lkdtm_do_action(enum ctype which)
case CT_WRITE_KERN:
lkdtm_WRITE_KERN();
break;
- case CT_ATOMIC_UNDERFLOW: {
- atomic_t under = ATOMIC_INIT(INT_MIN);
-
- pr_info("attempting good atomic increment\n");
- atomic_inc(&under);
- atomic_dec(&under);
-
- pr_info("attempting bad atomic underflow\n");
- atomic_dec(&under);
+ case CT_ATOMIC_UNDERFLOW:
+ lkdtm_ATOMIC_UNDERFLOW();
+ break;
+ case CT_ATOMIC_OVERFLOW:
+ lkdtm_ATOMIC_OVERFLOW();
break;
- }
- case CT_ATOMIC_OVERFLOW: {
- atomic_t over = ATOMIC_INIT(INT_MAX);
-
- pr_info("attempting good atomic decrement\n");
- atomic_dec(&over);
- atomic_inc(&over);
-
- pr_info("attempting bad atomic overflow\n");
- atomic_inc(&over);
-
- return;
- }
case CT_USERCOPY_HEAP_SIZE_TO:
lkdtm_USERCOPY_HEAP_SIZE_TO();
break;
@@ -786,6 +719,7 @@ static int __init lkdtm_module_init(void)
int i;
/* Handle test-specific initialization. */
+ lkdtm_bugs_init(&recur_count);
lkdtm_perms_init();
lkdtm_usercopy_init();
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/12] lkdtm: remove intentional off-by-one array access
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (5 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 06/12] lkdtm: split remaining logic bug " Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 08/12] lkdtm: rename "count" to "crash_count" Kees Cook
` (4 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
There wasn't a good reason for keeping the enum and the names out of sync
by 1 position just to avoid "NONE" and "INVALID" from being in the string
lists.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index e0f10131511f..4f9d2f32c88b 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -111,6 +111,7 @@ enum ctype {
};
static char* cp_name[] = {
+ "INVALID",
"INT_HARDWARE_ENTRY",
"INT_HW_IRQ_EN",
"INT_TASKLET_ENTRY",
@@ -123,6 +124,7 @@ static char* cp_name[] = {
};
static char* cp_type[] = {
+ "NONE",
"PANIC",
"BUG",
"WARNING",
@@ -257,7 +259,7 @@ static enum ctype parse_cp_type(const char *what, size_t count)
for (i = 0; i < ARRAY_SIZE(cp_type); i++) {
if (!strcmp(what, cp_type[i]))
- return i + 1;
+ return i;
}
return CT_NONE;
@@ -266,9 +268,9 @@ static enum ctype parse_cp_type(const char *what, size_t count)
static const char *cp_type_to_str(enum ctype type)
{
if (type == CT_NONE || type < 0 || type > ARRAY_SIZE(cp_type))
- return "None";
+ return "NONE";
- return cp_type[type - 1];
+ return cp_type[type];
}
static const char *cp_name_to_str(enum cname name)
@@ -276,7 +278,7 @@ static const char *cp_name_to_str(enum cname name)
if (name == CN_INVALID || name < 0 || name > ARRAY_SIZE(cp_name))
return "INVALID";
- return cp_name[name - 1];
+ return cp_name[name];
}
@@ -304,9 +306,13 @@ static int lkdtm_parse_commandline(void)
if (cptype == CT_NONE)
return -EINVAL;
+ /* Refuse INVALID as a selectable crashpoint name. */
+ if (!strcmp(cpoint_name, "INVALID"))
+ return -EINVAL;
+
for (i = 0; i < ARRAY_SIZE(cp_name); i++) {
if (!strcmp(cpoint_name, cp_name[i])) {
- cpoint = i + 1;
+ cpoint = i;
return 0;
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/12] lkdtm: rename "count" to "crash_count"
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (6 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 07/12] lkdtm: remove intentional off-by-one array access Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 09/12] lkdtm: rename globals for clarity Kees Cook
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
The "count" variable name was not easy to understand, since it was regularly
obscured by local variables of the same name, and it's purpose wasn't clear.
This renames it (and its lock) to "crash_count", which is more readable.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 4f9d2f32c88b..fa7335ee08de 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -51,11 +51,6 @@
#include "lkdtm.h"
-#define DEFAULT_COUNT 10
-
-static int count = DEFAULT_COUNT;
-static DEFINE_SPINLOCK(count_lock);
-
enum cname {
CN_INVALID,
CN_INT_HARDWARE_ENTRY,
@@ -169,10 +164,13 @@ static struct jprobe lkdtm;
static int lkdtm_parse_commandline(void);
static void lkdtm_handler(void);
+#define DEFAULT_COUNT 10
static char* cpoint_name;
static char* cpoint_type;
static int cpoint_count = DEFAULT_COUNT;
static int recur_count = -1;
+static int crash_count = DEFAULT_COUNT;
+static DEFINE_SPINLOCK(crash_count_lock);
static enum cname cpoint = CN_INVALID;
static enum ctype cptype = CT_NONE;
@@ -290,9 +288,9 @@ static int lkdtm_parse_commandline(void)
if (cpoint_count < 1 || recur_count < 1)
return -EINVAL;
- spin_lock_irqsave(&count_lock, flags);
- count = cpoint_count;
- spin_unlock_irqrestore(&count_lock, flags);
+ spin_lock_irqsave(&crash_count_lock, flags);
+ crash_count = cpoint_count;
+ spin_unlock_irqrestore(&crash_count_lock, flags);
/* No special parameters */
if (!cpoint_type && !cpoint_name)
@@ -447,16 +445,16 @@ static void lkdtm_handler(void)
unsigned long flags;
bool do_it = false;
- spin_lock_irqsave(&count_lock, flags);
- count--;
+ spin_lock_irqsave(&crash_count_lock, flags);
+ crash_count--;
pr_info("Crash point %s of type %s hit, trigger in %d rounds\n",
- cp_name_to_str(cpoint), cp_type_to_str(cptype), count);
+ cp_name_to_str(cpoint), cp_type_to_str(cptype), crash_count);
- if (count == 0) {
+ if (crash_count == 0) {
do_it = true;
- count = cpoint_count;
+ crash_count = cpoint_count;
}
- spin_unlock_irqrestore(&count_lock, flags);
+ spin_unlock_irqrestore(&crash_count_lock, flags);
if (do_it)
lkdtm_do_action(cptype);
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/12] lkdtm: rename globals for clarity
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (7 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 08/12] lkdtm: rename "count" to "crash_count" Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 10/12] lkdtm: reorganize module paramaters Kees Cook
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
The global variables used to track the active crashpoint and crashtype
are hard to distinguish from local variable names, so add a "lkdtm_"
prefix to them (or in the case of "lkdtm", add a "_jprobe" suffix).
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 75 ++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index fa7335ee08de..31b22f3b5fae 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -159,7 +159,7 @@ static char* cp_type[] = {
"USERCOPY_KERNEL",
};
-static struct jprobe lkdtm;
+static struct jprobe lkdtm_jprobe;
static int lkdtm_parse_commandline(void);
static void lkdtm_handler(void);
@@ -172,8 +172,8 @@ static int recur_count = -1;
static int crash_count = DEFAULT_COUNT;
static DEFINE_SPINLOCK(crash_count_lock);
-static enum cname cpoint = CN_INVALID;
-static enum ctype cptype = CT_NONE;
+static enum cname lkdtm_crashpoint = CN_INVALID;
+static enum ctype lkdtm_crashtype = CT_NONE;
module_param(recur_count, int, 0644);
MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test");
@@ -300,8 +300,8 @@ static int lkdtm_parse_commandline(void)
if (!cpoint_type || !cpoint_name)
return -EINVAL;
- cptype = parse_cp_type(cpoint_type, strlen(cpoint_type));
- if (cptype == CT_NONE)
+ lkdtm_crashtype = parse_cp_type(cpoint_type, strlen(cpoint_type));
+ if (lkdtm_crashtype == CT_NONE)
return -EINVAL;
/* Refuse INVALID as a selectable crashpoint name. */
@@ -310,7 +310,7 @@ static int lkdtm_parse_commandline(void)
for (i = 0; i < ARRAY_SIZE(cp_name); i++) {
if (!strcmp(cpoint_name, cp_name[i])) {
- cpoint = i;
+ lkdtm_crashpoint = i;
return 0;
}
}
@@ -448,7 +448,8 @@ static void lkdtm_handler(void)
spin_lock_irqsave(&crash_count_lock, flags);
crash_count--;
pr_info("Crash point %s of type %s hit, trigger in %d rounds\n",
- cp_name_to_str(cpoint), cp_type_to_str(cptype), crash_count);
+ cp_name_to_str(lkdtm_crashpoint),
+ cp_type_to_str(lkdtm_crashtype), crash_count);
if (crash_count == 0) {
do_it = true;
@@ -457,53 +458,53 @@ static void lkdtm_handler(void)
spin_unlock_irqrestore(&crash_count_lock, flags);
if (do_it)
- lkdtm_do_action(cptype);
+ lkdtm_do_action(lkdtm_crashtype);
}
static int lkdtm_register_cpoint(enum cname which)
{
int ret;
- cpoint = CN_INVALID;
- if (lkdtm.entry != NULL)
- unregister_jprobe(&lkdtm);
+ lkdtm_crashpoint = CN_INVALID;
+ if (lkdtm_jprobe.entry != NULL)
+ unregister_jprobe(&lkdtm_jprobe);
switch (which) {
case CN_DIRECT:
- lkdtm_do_action(cptype);
+ lkdtm_do_action(lkdtm_crashtype);
return 0;
case CN_INT_HARDWARE_ENTRY:
- lkdtm.kp.symbol_name = "do_IRQ";
- lkdtm.entry = (kprobe_opcode_t*) jp_do_irq;
+ lkdtm_jprobe.kp.symbol_name = "do_IRQ";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_do_irq;
break;
case CN_INT_HW_IRQ_EN:
- lkdtm.kp.symbol_name = "handle_IRQ_event";
- lkdtm.entry = (kprobe_opcode_t*) jp_handle_irq_event;
+ lkdtm_jprobe.kp.symbol_name = "handle_IRQ_event";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_handle_irq_event;
break;
case CN_INT_TASKLET_ENTRY:
- lkdtm.kp.symbol_name = "tasklet_action";
- lkdtm.entry = (kprobe_opcode_t*) jp_tasklet_action;
+ lkdtm_jprobe.kp.symbol_name = "tasklet_action";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_tasklet_action;
break;
case CN_FS_DEVRW:
- lkdtm.kp.symbol_name = "ll_rw_block";
- lkdtm.entry = (kprobe_opcode_t*) jp_ll_rw_block;
+ lkdtm_jprobe.kp.symbol_name = "ll_rw_block";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_ll_rw_block;
break;
case CN_MEM_SWAPOUT:
- lkdtm.kp.symbol_name = "shrink_inactive_list";
- lkdtm.entry = (kprobe_opcode_t*) jp_shrink_inactive_list;
+ lkdtm_jprobe.kp.symbol_name = "shrink_inactive_list";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_shrink_inactive_list;
break;
case CN_TIMERADD:
- lkdtm.kp.symbol_name = "hrtimer_start";
- lkdtm.entry = (kprobe_opcode_t*) jp_hrtimer_start;
+ lkdtm_jprobe.kp.symbol_name = "hrtimer_start";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_hrtimer_start;
break;
case CN_SCSI_DISPATCH_CMD:
- lkdtm.kp.symbol_name = "scsi_dispatch_cmd";
- lkdtm.entry = (kprobe_opcode_t*) jp_scsi_dispatch_cmd;
+ lkdtm_jprobe.kp.symbol_name = "scsi_dispatch_cmd";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_scsi_dispatch_cmd;
break;
case CN_IDE_CORE_CP:
#ifdef CONFIG_IDE
- lkdtm.kp.symbol_name = "generic_ide_ioctl";
- lkdtm.entry = (kprobe_opcode_t*) jp_generic_ide_ioctl;
+ lkdtm_jprobe.kp.symbol_name = "generic_ide_ioctl";
+ lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_generic_ide_ioctl;
#else
pr_info("Crash point not available\n");
return -EINVAL;
@@ -514,10 +515,10 @@ static int lkdtm_register_cpoint(enum cname which)
return -EINVAL;
}
- cpoint = which;
- if ((ret = register_jprobe(&lkdtm)) < 0) {
+ lkdtm_crashpoint = which;
+ if ((ret = register_jprobe(&lkdtm_jprobe)) < 0) {
pr_info("Couldn't register jprobe\n");
- cpoint = CN_INVALID;
+ lkdtm_crashpoint = CN_INVALID;
}
return ret;
@@ -543,10 +544,10 @@ static ssize_t do_register_entry(enum cname which, struct file *f,
buf[count] = '\0';
strim(buf);
- cptype = parse_cp_type(buf, count);
+ lkdtm_crashtype = parse_cp_type(buf, count);
free_page((unsigned long) buf);
- if (cptype == CT_NONE)
+ if (lkdtm_crashtype == CT_NONE)
return -EINVAL;
err = lkdtm_register_cpoint(which);
@@ -755,10 +756,10 @@ static int __init lkdtm_module_init(void)
goto out_err;
}
- if (cpoint != CN_INVALID && cptype != CT_NONE) {
- ret = lkdtm_register_cpoint(cpoint);
+ if (lkdtm_crashpoint != CN_INVALID && lkdtm_crashtype != CT_NONE) {
+ ret = lkdtm_register_cpoint(lkdtm_crashpoint);
if (ret < 0) {
- pr_info("Invalid crash point %d\n", cpoint);
+ pr_info("Invalid crash point %d\n", lkdtm_crashpoint);
goto out_err;
}
pr_info("Crash point %s of type %s registered\n",
@@ -781,7 +782,7 @@ static void __exit lkdtm_module_exit(void)
/* Handle test-specific clean-up. */
lkdtm_usercopy_exit();
- unregister_jprobe(&lkdtm);
+ unregister_jprobe(&lkdtm_jprobe);
pr_info("Crash point unregistered\n");
}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/12] lkdtm: reorganize module paramaters
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (8 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 09/12] lkdtm: rename globals for clarity Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 11/12] lkdtm: move jprobe entry points to start of source Kees Cook
2016-07-06 22:33 ` [PATCH 12/12] lkdtm: use struct arrays instead of enums Kees Cook
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This reorganizes module parameters and global variables in the source
so they're grouped together with comments. Also moves early function
declarations to the top of the file.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 31b22f3b5fae..ff28bd03a9e8 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -51,6 +51,11 @@
#include "lkdtm.h"
+#define DEFAULT_COUNT 10
+
+static int lkdtm_parse_commandline(void);
+static void lkdtm_handler(void);
+
enum cname {
CN_INVALID,
CN_INT_HARDWARE_ENTRY,
@@ -159,29 +164,30 @@ static char* cp_type[] = {
"USERCOPY_KERNEL",
};
+/* Global jprobe entry and crashtype. */
static struct jprobe lkdtm_jprobe;
+static enum cname lkdtm_crashpoint = CN_INVALID;
+static enum ctype lkdtm_crashtype = CT_NONE;
-static int lkdtm_parse_commandline(void);
-static void lkdtm_handler(void);
-
-#define DEFAULT_COUNT 10
-static char* cpoint_name;
-static char* cpoint_type;
-static int cpoint_count = DEFAULT_COUNT;
-static int recur_count = -1;
+/* Global crash counter and spinlock. */
static int crash_count = DEFAULT_COUNT;
static DEFINE_SPINLOCK(crash_count_lock);
-static enum cname lkdtm_crashpoint = CN_INVALID;
-static enum ctype lkdtm_crashtype = CT_NONE;
-
+/* Module parameters */
+static int recur_count = -1;
module_param(recur_count, int, 0644);
MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test");
+
+static char* cpoint_name;
module_param(cpoint_name, charp, 0444);
MODULE_PARM_DESC(cpoint_name, " Crash Point, where kernel is to be crashed");
+
+static char* cpoint_type;
module_param(cpoint_type, charp, 0444);
MODULE_PARM_DESC(cpoint_type, " Crash Point Type, action to be taken on "\
"hitting the crash point");
+
+static int cpoint_count = DEFAULT_COUNT;
module_param(cpoint_count, int, 0644);
MODULE_PARM_DESC(cpoint_count, " Crash Point Count, number of times the "\
"crash point is to be hit to trigger action");
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 11/12] lkdtm: move jprobe entry points to start of source
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (9 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 10/12] lkdtm: reorganize module paramaters Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-06 22:33 ` [PATCH 12/12] lkdtm: use struct arrays instead of enums Kees Cook
11 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
In preparation of referencing the jprobe entry points in a structure,
this moves them to the start of the source since they operate mostly
separately from everything else.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 129 +++++++++++++++++++++++-----------------------
1 file changed, 65 insertions(+), 64 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index ff28bd03a9e8..8b90220216e2 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -56,6 +56,71 @@
static int lkdtm_parse_commandline(void);
static void lkdtm_handler(void);
+/* jprobe entry point handlers. */
+static unsigned int jp_do_irq(unsigned int irq)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+
+static irqreturn_t jp_handle_irq_event(unsigned int irq,
+ struct irqaction *action)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+
+static void jp_tasklet_action(struct softirq_action *a)
+{
+ lkdtm_handler();
+ jprobe_return();
+}
+
+static void jp_ll_rw_block(int rw, int nr, struct buffer_head *bhs[])
+{
+ lkdtm_handler();
+ jprobe_return();
+}
+
+struct scan_control;
+
+static unsigned long jp_shrink_inactive_list(unsigned long max_scan,
+ struct zone *zone,
+ struct scan_control *sc)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+
+static int jp_hrtimer_start(struct hrtimer *timer, ktime_t tim,
+ const enum hrtimer_mode mode)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+
+static int jp_scsi_dispatch_cmd(struct scsi_cmnd *cmd)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+
+#ifdef CONFIG_IDE
+static int jp_generic_ide_ioctl(ide_drive_t *drive, struct file *file,
+ struct block_device *bdev, unsigned int cmd,
+ unsigned long arg)
+{
+ lkdtm_handler();
+ jprobe_return();
+ return 0;
+}
+#endif
+
enum cname {
CN_INVALID,
CN_INT_HARDWARE_ENTRY,
@@ -192,70 +257,6 @@ module_param(cpoint_count, int, 0644);
MODULE_PARM_DESC(cpoint_count, " Crash Point Count, number of times the "\
"crash point is to be hit to trigger action");
-static unsigned int jp_do_irq(unsigned int irq)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-
-static irqreturn_t jp_handle_irq_event(unsigned int irq,
- struct irqaction *action)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-
-static void jp_tasklet_action(struct softirq_action *a)
-{
- lkdtm_handler();
- jprobe_return();
-}
-
-static void jp_ll_rw_block(int rw, int nr, struct buffer_head *bhs[])
-{
- lkdtm_handler();
- jprobe_return();
-}
-
-struct scan_control;
-
-static unsigned long jp_shrink_inactive_list(unsigned long max_scan,
- struct zone *zone,
- struct scan_control *sc)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-
-static int jp_hrtimer_start(struct hrtimer *timer, ktime_t tim,
- const enum hrtimer_mode mode)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-
-static int jp_scsi_dispatch_cmd(struct scsi_cmnd *cmd)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-
-#ifdef CONFIG_IDE
-static int jp_generic_ide_ioctl(ide_drive_t *drive, struct file *file,
- struct block_device *bdev, unsigned int cmd,
- unsigned long arg)
-{
- lkdtm_handler();
- jprobe_return();
- return 0;
-}
-#endif
-
/* Return the crashpoint number or NONE if the name is invalid */
static enum ctype parse_cp_type(const char *what, size_t count)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 12/12] lkdtm: use struct arrays instead of enums
2016-07-06 22:33 [PATCH 00/12] lkdtm: use struct arrays instead of enums Kees Cook
` (10 preceding siblings ...)
2016-07-06 22:33 ` [PATCH 11/12] lkdtm: move jprobe entry points to start of source Kees Cook
@ 2016-07-06 22:33 ` Kees Cook
2016-07-15 11:35 ` [PATCH] lkdtm: hide unused functions Arnd Bergmann
11 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2016-07-06 22:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Kees Cook, Greg Kroah-Hartman, Arnd Bergmann
This removes the use of enums in favor of much more readable and compact
structure arrays. This requires changing all the enum passing to pointers
instead, but the results are much cleaner.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm_core.c | 668 ++++++++++++++--------------------------------
1 file changed, 205 insertions(+), 463 deletions(-)
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index 8b90220216e2..de29a339242a 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -53,8 +53,16 @@
#define DEFAULT_COUNT 10
-static int lkdtm_parse_commandline(void);
static void lkdtm_handler(void);
+static int lkdtm_debugfs_open(struct inode *inode, struct file *file);
+static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
+ size_t count, loff_t *off);
+static ssize_t direct_entry(struct file *f, const char __user *user_buf,
+ size_t count, loff_t *off);
+static ssize_t lkdtm_debugfs_entry(struct file *f,
+ const char __user *user_buf,
+ size_t count, loff_t *off);
+
/* jprobe entry point handlers. */
static unsigned int jp_do_irq(unsigned int irq)
@@ -121,118 +129,114 @@ static int jp_generic_ide_ioctl(ide_drive_t *drive, struct file *file,
}
#endif
-enum cname {
- CN_INVALID,
- CN_INT_HARDWARE_ENTRY,
- CN_INT_HW_IRQ_EN,
- CN_INT_TASKLET_ENTRY,
- CN_FS_DEVRW,
- CN_MEM_SWAPOUT,
- CN_TIMERADD,
- CN_SCSI_DISPATCH_CMD,
- CN_IDE_CORE_CP,
- CN_DIRECT,
+
+/* Crash points */
+struct crashpoint {
+ const char *name;
+ const struct file_operations fops;
+ struct jprobe jprobe;
};
-enum ctype {
- CT_NONE,
- CT_PANIC,
- CT_BUG,
- CT_WARNING,
- CT_EXCEPTION,
- CT_LOOP,
- CT_OVERFLOW,
- CT_CORRUPT_STACK,
- CT_UNALIGNED_LOAD_STORE_WRITE,
- CT_OVERWRITE_ALLOCATION,
- CT_WRITE_AFTER_FREE,
- CT_READ_AFTER_FREE,
- CT_WRITE_BUDDY_AFTER_FREE,
- CT_READ_BUDDY_AFTER_FREE,
- CT_SOFTLOCKUP,
- CT_HARDLOCKUP,
- CT_SPINLOCKUP,
- CT_HUNG_TASK,
- CT_EXEC_DATA,
- CT_EXEC_STACK,
- CT_EXEC_KMALLOC,
- CT_EXEC_VMALLOC,
- CT_EXEC_RODATA,
- CT_EXEC_USERSPACE,
- CT_ACCESS_USERSPACE,
- CT_WRITE_RO,
- CT_WRITE_RO_AFTER_INIT,
- CT_WRITE_KERN,
- CT_ATOMIC_UNDERFLOW,
- CT_ATOMIC_OVERFLOW,
- CT_USERCOPY_HEAP_SIZE_TO,
- CT_USERCOPY_HEAP_SIZE_FROM,
- CT_USERCOPY_HEAP_FLAG_TO,
- CT_USERCOPY_HEAP_FLAG_FROM,
- CT_USERCOPY_STACK_FRAME_TO,
- CT_USERCOPY_STACK_FRAME_FROM,
- CT_USERCOPY_STACK_BEYOND,
- CT_USERCOPY_KERNEL,
+#define CRASHPOINT(_name, _write, _symbol, _entry) \
+ { \
+ .name = _name, \
+ .fops = { \
+ .read = lkdtm_debugfs_read, \
+ .llseek = generic_file_llseek, \
+ .open = lkdtm_debugfs_open, \
+ .write = _write, \
+ }, \
+ .jprobe = { \
+ .kp.symbol_name = _symbol, \
+ .entry = (kprobe_opcode_t *)_entry, \
+ }, \
+ }
+
+/* Define the possible places where we can trigger a crash point. */
+struct crashpoint crashpoints[] = {
+ CRASHPOINT("DIRECT", direct_entry,
+ NULL, NULL),
+#ifdef CONFIG_KPROBES
+ CRASHPOINT("INT_HARDWARE_ENTRY", lkdtm_debugfs_entry,
+ "do_IRQ", jp_do_irq),
+ CRASHPOINT("INT_HW_IRQ_EN", lkdtm_debugfs_entry,
+ "handle_IRQ_event", jp_handle_irq_event),
+ CRASHPOINT("INT_TASKLET_ENTRY", lkdtm_debugfs_entry,
+ "tasklet_action", jp_tasklet_action),
+ CRASHPOINT("FS_DEVRW", lkdtm_debugfs_entry,
+ "ll_rw_block", jp_ll_rw_block),
+ CRASHPOINT("MEM_SWAPOUT", lkdtm_debugfs_entry,
+ "shrink_inactive_list", jp_shrink_inactive_list),
+ CRASHPOINT("TIMERADD", lkdtm_debugfs_entry,
+ "hrtimer_start", jp_hrtimer_start),
+ CRASHPOINT("SCSI_DISPATCH_CMD", lkdtm_debugfs_entry,
+ "scsi_dispatch_cmd", jp_scsi_dispatch_cmd),
+# ifdef CONFIG_IDE
+ CRASHPOINT("IDE_CORE_CP", lkdtm_debugfs_entry,
+ "generic_ide_ioctl", jp_generic_ide_ioctl),
+# endif
+#endif
};
-static char* cp_name[] = {
- "INVALID",
- "INT_HARDWARE_ENTRY",
- "INT_HW_IRQ_EN",
- "INT_TASKLET_ENTRY",
- "FS_DEVRW",
- "MEM_SWAPOUT",
- "TIMERADD",
- "SCSI_DISPATCH_CMD",
- "IDE_CORE_CP",
- "DIRECT",
+
+/* Crash types. */
+struct crashtype {
+ const char *name;
+ void (*func)(void);
};
-static char* cp_type[] = {
- "NONE",
- "PANIC",
- "BUG",
- "WARNING",
- "EXCEPTION",
- "LOOP",
- "OVERFLOW",
- "CORRUPT_STACK",
- "UNALIGNED_LOAD_STORE_WRITE",
- "OVERWRITE_ALLOCATION",
- "WRITE_AFTER_FREE",
- "READ_AFTER_FREE",
- "WRITE_BUDDY_AFTER_FREE",
- "READ_BUDDY_AFTER_FREE",
- "SOFTLOCKUP",
- "HARDLOCKUP",
- "SPINLOCKUP",
- "HUNG_TASK",
- "EXEC_DATA",
- "EXEC_STACK",
- "EXEC_KMALLOC",
- "EXEC_VMALLOC",
- "EXEC_RODATA",
- "EXEC_USERSPACE",
- "ACCESS_USERSPACE",
- "WRITE_RO",
- "WRITE_RO_AFTER_INIT",
- "WRITE_KERN",
- "ATOMIC_UNDERFLOW",
- "ATOMIC_OVERFLOW",
- "USERCOPY_HEAP_SIZE_TO",
- "USERCOPY_HEAP_SIZE_FROM",
- "USERCOPY_HEAP_FLAG_TO",
- "USERCOPY_HEAP_FLAG_FROM",
- "USERCOPY_STACK_FRAME_TO",
- "USERCOPY_STACK_FRAME_FROM",
- "USERCOPY_STACK_BEYOND",
- "USERCOPY_KERNEL",
+#define CRASHTYPE(_name) \
+ { \
+ .name = __stringify(_name), \
+ .func = lkdtm_ ## _name, \
+ }
+
+/* Define the possible types of crashes that can be triggered. */
+struct crashtype crashtypes[] = {
+ CRASHTYPE(PANIC),
+ CRASHTYPE(BUG),
+ CRASHTYPE(WARNING),
+ CRASHTYPE(EXCEPTION),
+ CRASHTYPE(LOOP),
+ CRASHTYPE(OVERFLOW),
+ CRASHTYPE(CORRUPT_STACK),
+ CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE),
+ CRASHTYPE(OVERWRITE_ALLOCATION),
+ CRASHTYPE(WRITE_AFTER_FREE),
+ CRASHTYPE(READ_AFTER_FREE),
+ CRASHTYPE(WRITE_BUDDY_AFTER_FREE),
+ CRASHTYPE(READ_BUDDY_AFTER_FREE),
+ CRASHTYPE(SOFTLOCKUP),
+ CRASHTYPE(HARDLOCKUP),
+ CRASHTYPE(SPINLOCKUP),
+ CRASHTYPE(HUNG_TASK),
+ CRASHTYPE(EXEC_DATA),
+ CRASHTYPE(EXEC_STACK),
+ CRASHTYPE(EXEC_KMALLOC),
+ CRASHTYPE(EXEC_VMALLOC),
+ CRASHTYPE(EXEC_RODATA),
+ CRASHTYPE(EXEC_USERSPACE),
+ CRASHTYPE(ACCESS_USERSPACE),
+ CRASHTYPE(WRITE_RO),
+ CRASHTYPE(WRITE_RO_AFTER_INIT),
+ CRASHTYPE(WRITE_KERN),
+ CRASHTYPE(ATOMIC_UNDERFLOW),
+ CRASHTYPE(ATOMIC_OVERFLOW),
+ CRASHTYPE(USERCOPY_HEAP_SIZE_TO),
+ CRASHTYPE(USERCOPY_HEAP_SIZE_FROM),
+ CRASHTYPE(USERCOPY_HEAP_FLAG_TO),
+ CRASHTYPE(USERCOPY_HEAP_FLAG_FROM),
+ CRASHTYPE(USERCOPY_STACK_FRAME_TO),
+ CRASHTYPE(USERCOPY_STACK_FRAME_FROM),
+ CRASHTYPE(USERCOPY_STACK_BEYOND),
+ CRASHTYPE(USERCOPY_KERNEL),
};
+
/* Global jprobe entry and crashtype. */
-static struct jprobe lkdtm_jprobe;
-static enum cname lkdtm_crashpoint = CN_INVALID;
-static enum ctype lkdtm_crashtype = CT_NONE;
+static struct jprobe *lkdtm_jprobe;
+struct crashpoint *lkdtm_crashpoint;
+struct crashtype *lkdtm_crashtype;
/* Global crash counter and spinlock. */
static int crash_count = DEFAULT_COUNT;
@@ -257,206 +261,42 @@ module_param(cpoint_count, int, 0644);
MODULE_PARM_DESC(cpoint_count, " Crash Point Count, number of times the "\
"crash point is to be hit to trigger action");
-/* Return the crashpoint number or NONE if the name is invalid */
-static enum ctype parse_cp_type(const char *what, size_t count)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(cp_type); i++) {
- if (!strcmp(what, cp_type[i]))
- return i;
- }
-
- return CT_NONE;
-}
-
-static const char *cp_type_to_str(enum ctype type)
-{
- if (type == CT_NONE || type < 0 || type > ARRAY_SIZE(cp_type))
- return "NONE";
-
- return cp_type[type];
-}
-
-static const char *cp_name_to_str(enum cname name)
-{
- if (name == CN_INVALID || name < 0 || name > ARRAY_SIZE(cp_name))
- return "INVALID";
-
- return cp_name[name];
-}
-
-static int lkdtm_parse_commandline(void)
+/* Return the crashtype number or NULL if the name is invalid */
+static struct crashtype *find_crashtype(const char *name)
{
int i;
- unsigned long flags;
-
- if (cpoint_count < 1 || recur_count < 1)
- return -EINVAL;
-
- spin_lock_irqsave(&crash_count_lock, flags);
- crash_count = cpoint_count;
- spin_unlock_irqrestore(&crash_count_lock, flags);
- /* No special parameters */
- if (!cpoint_type && !cpoint_name)
- return 0;
-
- /* Neither or both of these need to be set */
- if (!cpoint_type || !cpoint_name)
- return -EINVAL;
-
- lkdtm_crashtype = parse_cp_type(cpoint_type, strlen(cpoint_type));
- if (lkdtm_crashtype == CT_NONE)
- return -EINVAL;
-
- /* Refuse INVALID as a selectable crashpoint name. */
- if (!strcmp(cpoint_name, "INVALID"))
- return -EINVAL;
-
- for (i = 0; i < ARRAY_SIZE(cp_name); i++) {
- if (!strcmp(cpoint_name, cp_name[i])) {
- lkdtm_crashpoint = i;
- return 0;
- }
+ for (i = 0; i < ARRAY_SIZE(crashtypes); i++) {
+ if (!strcmp(name, crashtypes[i].name))
+ return &crashtypes[i];
}
- /* Could not find a valid crash point */
- return -EINVAL;
+ return NULL;
}
-static void lkdtm_do_action(enum ctype which)
+/*
+ * This is forced noinline just so it distinctly shows up in the stackdump
+ * which makes validation of expected lkdtm crashes easier.
+ */
+static noinline void lkdtm_do_action(struct crashtype *crashtype)
{
- switch (which) {
- case CT_PANIC:
- lkdtm_PANIC();
- break;
- case CT_BUG:
- lkdtm_BUG();
- break;
- case CT_WARNING:
- lkdtm_WARNING();
- break;
- case CT_EXCEPTION:
- lkdtm_EXCEPTION();
- break;
- case CT_LOOP:
- lkdtm_LOOP();
- break;
- case CT_OVERFLOW:
- lkdtm_OVERFLOW();
- break;
- case CT_CORRUPT_STACK:
- lkdtm_CORRUPT_STACK();
- break;
- case CT_UNALIGNED_LOAD_STORE_WRITE:
- lkdtm_UNALIGNED_LOAD_STORE_WRITE();
- break;
- case CT_OVERWRITE_ALLOCATION:
- lkdtm_OVERWRITE_ALLOCATION();
- break;
- case CT_WRITE_AFTER_FREE:
- lkdtm_WRITE_AFTER_FREE();
- break;
- case CT_READ_AFTER_FREE:
- lkdtm_READ_AFTER_FREE();
- break;
- case CT_WRITE_BUDDY_AFTER_FREE:
- lkdtm_WRITE_BUDDY_AFTER_FREE();
- break;
- case CT_READ_BUDDY_AFTER_FREE:
- lkdtm_READ_BUDDY_AFTER_FREE();
- break;
- case CT_SOFTLOCKUP:
- lkdtm_SOFTLOCKUP();
- break;
- case CT_HARDLOCKUP:
- lkdtm_HARDLOCKUP();
- break;
- case CT_SPINLOCKUP:
- lkdtm_SPINLOCKUP();
- break;
- case CT_HUNG_TASK:
- lkdtm_HUNG_TASK();
- break;
- case CT_EXEC_DATA:
- lkdtm_EXEC_DATA();
- break;
- case CT_EXEC_STACK:
- lkdtm_EXEC_STACK();
- break;
- case CT_EXEC_KMALLOC:
- lkdtm_EXEC_KMALLOC();
- break;
- case CT_EXEC_VMALLOC:
- lkdtm_EXEC_VMALLOC();
- break;
- case CT_EXEC_RODATA:
- lkdtm_EXEC_RODATA();
- break;
- case CT_EXEC_USERSPACE:
- lkdtm_EXEC_USERSPACE();
- break;
- case CT_ACCESS_USERSPACE:
- lkdtm_ACCESS_USERSPACE();
- break;
- case CT_WRITE_RO:
- lkdtm_WRITE_RO();
- break;
- case CT_WRITE_RO_AFTER_INIT:
- lkdtm_WRITE_RO_AFTER_INIT();
- break;
- case CT_WRITE_KERN:
- lkdtm_WRITE_KERN();
- break;
- case CT_ATOMIC_UNDERFLOW:
- lkdtm_ATOMIC_UNDERFLOW();
- break;
- case CT_ATOMIC_OVERFLOW:
- lkdtm_ATOMIC_OVERFLOW();
- break;
- case CT_USERCOPY_HEAP_SIZE_TO:
- lkdtm_USERCOPY_HEAP_SIZE_TO();
- break;
- case CT_USERCOPY_HEAP_SIZE_FROM:
- lkdtm_USERCOPY_HEAP_SIZE_FROM();
- break;
- case CT_USERCOPY_HEAP_FLAG_TO:
- lkdtm_USERCOPY_HEAP_FLAG_TO();
- break;
- case CT_USERCOPY_HEAP_FLAG_FROM:
- lkdtm_USERCOPY_HEAP_FLAG_FROM();
- break;
- case CT_USERCOPY_STACK_FRAME_TO:
- lkdtm_USERCOPY_STACK_FRAME_TO();
- break;
- case CT_USERCOPY_STACK_FRAME_FROM:
- lkdtm_USERCOPY_STACK_FRAME_FROM();
- break;
- case CT_USERCOPY_STACK_BEYOND:
- lkdtm_USERCOPY_STACK_BEYOND();
- break;
- case CT_USERCOPY_KERNEL:
- lkdtm_USERCOPY_KERNEL();
- break;
- case CT_NONE:
- default:
- break;
- }
-
+ BUG_ON(!crashtype || !crashtype->func);
+ crashtype->func();
}
+/* Called by jprobe entry points. */
static void lkdtm_handler(void)
{
unsigned long flags;
bool do_it = false;
+ BUG_ON(!lkdtm_crashpoint || !lkdtm_crashtype);
+
spin_lock_irqsave(&crash_count_lock, flags);
crash_count--;
pr_info("Crash point %s of type %s hit, trigger in %d rounds\n",
- cp_name_to_str(lkdtm_crashpoint),
- cp_type_to_str(lkdtm_crashtype), crash_count);
+ lkdtm_crashpoint->name, lkdtm_crashtype->name, crash_count);
if (crash_count == 0) {
do_it = true;
@@ -468,72 +308,41 @@ static void lkdtm_handler(void)
lkdtm_do_action(lkdtm_crashtype);
}
-static int lkdtm_register_cpoint(enum cname which)
+static int lkdtm_register_cpoint(struct crashpoint *crashpoint,
+ struct crashtype *crashtype)
{
int ret;
- lkdtm_crashpoint = CN_INVALID;
- if (lkdtm_jprobe.entry != NULL)
- unregister_jprobe(&lkdtm_jprobe);
-
- switch (which) {
- case CN_DIRECT:
- lkdtm_do_action(lkdtm_crashtype);
+ /* If this doesn't have a symbol, just call immediately. */
+ if (!crashpoint->jprobe.kp.symbol_name) {
+ lkdtm_do_action(crashtype);
return 0;
- case CN_INT_HARDWARE_ENTRY:
- lkdtm_jprobe.kp.symbol_name = "do_IRQ";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_do_irq;
- break;
- case CN_INT_HW_IRQ_EN:
- lkdtm_jprobe.kp.symbol_name = "handle_IRQ_event";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_handle_irq_event;
- break;
- case CN_INT_TASKLET_ENTRY:
- lkdtm_jprobe.kp.symbol_name = "tasklet_action";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_tasklet_action;
- break;
- case CN_FS_DEVRW:
- lkdtm_jprobe.kp.symbol_name = "ll_rw_block";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_ll_rw_block;
- break;
- case CN_MEM_SWAPOUT:
- lkdtm_jprobe.kp.symbol_name = "shrink_inactive_list";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_shrink_inactive_list;
- break;
- case CN_TIMERADD:
- lkdtm_jprobe.kp.symbol_name = "hrtimer_start";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_hrtimer_start;
- break;
- case CN_SCSI_DISPATCH_CMD:
- lkdtm_jprobe.kp.symbol_name = "scsi_dispatch_cmd";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_scsi_dispatch_cmd;
- break;
- case CN_IDE_CORE_CP:
-#ifdef CONFIG_IDE
- lkdtm_jprobe.kp.symbol_name = "generic_ide_ioctl";
- lkdtm_jprobe.entry = (kprobe_opcode_t*) jp_generic_ide_ioctl;
-#else
- pr_info("Crash point not available\n");
- return -EINVAL;
-#endif
- break;
- default:
- pr_info("Invalid Crash Point\n");
- return -EINVAL;
}
- lkdtm_crashpoint = which;
- if ((ret = register_jprobe(&lkdtm_jprobe)) < 0) {
- pr_info("Couldn't register jprobe\n");
- lkdtm_crashpoint = CN_INVALID;
+ if (lkdtm_jprobe != NULL)
+ unregister_jprobe(lkdtm_jprobe);
+
+ lkdtm_crashpoint = crashpoint;
+ lkdtm_crashtype = crashtype;
+ lkdtm_jprobe = &crashpoint->jprobe;
+ ret = register_jprobe(lkdtm_jprobe);
+ if (ret < 0) {
+ pr_info("Couldn't register jprobe %s\n",
+ crashpoint->jprobe.kp.symbol_name);
+ lkdtm_jprobe = NULL;
+ lkdtm_crashpoint = NULL;
+ lkdtm_crashtype = NULL;
}
return ret;
}
-static ssize_t do_register_entry(enum cname which, struct file *f,
- const char __user *user_buf, size_t count, loff_t *off)
+static ssize_t lkdtm_debugfs_entry(struct file *f,
+ const char __user *user_buf,
+ size_t count, loff_t *off)
{
+ struct crashpoint *crashpoint = file_inode(f)->i_private;
+ struct crashtype *crashtype = NULL;
char *buf;
int err;
@@ -551,13 +360,13 @@ static ssize_t do_register_entry(enum cname which, struct file *f,
buf[count] = '\0';
strim(buf);
- lkdtm_crashtype = parse_cp_type(buf, count);
- free_page((unsigned long) buf);
+ crashtype = find_crashtype(buf);
+ free_page((unsigned long)buf);
- if (lkdtm_crashtype == CT_NONE)
+ if (!crashtype)
return -EINVAL;
- err = lkdtm_register_cpoint(which);
+ err = lkdtm_register_cpoint(crashpoint, crashtype);
if (err < 0)
return err;
@@ -578,8 +387,10 @@ static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
return -ENOMEM;
n = snprintf(buf, PAGE_SIZE, "Available crash types:\n");
- for (i = 0; i < ARRAY_SIZE(cp_type); i++)
- n += snprintf(buf + n, PAGE_SIZE - n, "%s\n", cp_type[i]);
+ for (i = 0; i < ARRAY_SIZE(crashtypes); i++) {
+ n += snprintf(buf + n, PAGE_SIZE - n, "%s\n",
+ crashtypes[i].name);
+ }
buf[n] = '\0';
out = simple_read_from_buffer(user_buf, count, off,
@@ -594,60 +405,11 @@ static int lkdtm_debugfs_open(struct inode *inode, struct file *file)
return 0;
}
-
-static ssize_t int_hardware_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_INT_HARDWARE_ENTRY, f, buf, count, off);
-}
-
-static ssize_t int_hw_irq_en(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_INT_HW_IRQ_EN, f, buf, count, off);
-}
-
-static ssize_t int_tasklet_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_INT_TASKLET_ENTRY, f, buf, count, off);
-}
-
-static ssize_t fs_devrw_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_FS_DEVRW, f, buf, count, off);
-}
-
-static ssize_t mem_swapout_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_MEM_SWAPOUT, f, buf, count, off);
-}
-
-static ssize_t timeradd_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_TIMERADD, f, buf, count, off);
-}
-
-static ssize_t scsi_dispatch_cmd_entry(struct file *f,
- const char __user *buf, size_t count, loff_t *off)
-{
- return do_register_entry(CN_SCSI_DISPATCH_CMD, f, buf, count, off);
-}
-
-static ssize_t ide_core_cp_entry(struct file *f, const char __user *buf,
- size_t count, loff_t *off)
-{
- return do_register_entry(CN_IDE_CORE_CP, f, buf, count, off);
-}
-
/* Special entry to just crash directly. Available without KPROBEs */
static ssize_t direct_entry(struct file *f, const char __user *user_buf,
size_t count, loff_t *off)
{
- enum ctype type;
+ struct crashtype *crashtype;
char *buf;
if (count >= PAGE_SIZE)
@@ -666,70 +428,57 @@ static ssize_t direct_entry(struct file *f, const char __user *user_buf,
buf[count] = '\0';
strim(buf);
- type = parse_cp_type(buf, count);
+ crashtype = find_crashtype(buf);
free_page((unsigned long) buf);
- if (type == CT_NONE)
+ if (!crashtype)
return -EINVAL;
- pr_info("Performing direct entry %s\n", cp_type_to_str(type));
- lkdtm_do_action(type);
+ pr_info("Performing direct entry %s\n", crashtype->name);
+ lkdtm_do_action(crashtype);
*off += count;
return count;
}
-struct crash_entry {
- const char *name;
- const struct file_operations fops;
-};
-
-static const struct crash_entry crash_entries[] = {
- {"DIRECT", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = direct_entry} },
- {"INT_HARDWARE_ENTRY", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = int_hardware_entry} },
- {"INT_HW_IRQ_EN", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = int_hw_irq_en} },
- {"INT_TASKLET_ENTRY", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = int_tasklet_entry} },
- {"FS_DEVRW", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = fs_devrw_entry} },
- {"MEM_SWAPOUT", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = mem_swapout_entry} },
- {"TIMERADD", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = timeradd_entry} },
- {"SCSI_DISPATCH_CMD", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = scsi_dispatch_cmd_entry} },
- {"IDE_CORE_CP", {.read = lkdtm_debugfs_read,
- .llseek = generic_file_llseek,
- .open = lkdtm_debugfs_open,
- .write = ide_core_cp_entry} },
-};
-
static struct dentry *lkdtm_debugfs_root;
static int __init lkdtm_module_init(void)
{
+ struct crashpoint *crashpoint = NULL;
+ struct crashtype *crashtype = NULL;
int ret = -EINVAL;
- int n_debugfs_entries = 1; /* Assume only the direct entry */
int i;
+ /* Neither or both of these need to be set */
+ if ((cpoint_type || cpoint_name) && !(cpoint_type && cpoint_name)) {
+ pr_err("Need both cpoint_type and cpoint_name or neither\n");
+ return -EINVAL;
+ }
+
+ if (cpoint_type) {
+ crashtype = find_crashtype(cpoint_type);
+ if (!crashtype) {
+ pr_err("Unknown crashtype '%s'\n", cpoint_type);
+ return -EINVAL;
+ }
+ }
+
+ if (cpoint_name) {
+ for (i = 0; i < ARRAY_SIZE(crashpoints); i++) {
+ if (!strcmp(cpoint_name, crashpoints[i].name))
+ crashpoint = &crashpoints[i];
+ }
+
+ /* Refuse unknown crashpoints. */
+ if (!crashpoint) {
+ pr_err("Invalid crashpoint %s\n", cpoint_name);
+ return -EINVAL;
+ }
+ }
+
+ /* Set crash count. */
+ crash_count = cpoint_count;
+
/* Handle test-specific initialization. */
lkdtm_bugs_init(&recur_count);
lkdtm_perms_init();
@@ -742,35 +491,28 @@ static int __init lkdtm_module_init(void)
return -ENODEV;
}
-#ifdef CONFIG_KPROBES
- n_debugfs_entries = ARRAY_SIZE(crash_entries);
-#endif
-
- for (i = 0; i < n_debugfs_entries; i++) {
- const struct crash_entry *cur = &crash_entries[i];
+ /* Install debugfs trigger files. */
+ for (i = 0; i < ARRAY_SIZE(crashpoints); i++) {
+ struct crashpoint *cur = &crashpoints[i];
struct dentry *de;
de = debugfs_create_file(cur->name, 0644, lkdtm_debugfs_root,
- NULL, &cur->fops);
+ cur, &cur->fops);
if (de == NULL) {
- pr_err("could not create %s\n", cur->name);
+ pr_err("could not create crashpoint %s\n", cur->name);
goto out_err;
}
}
- if (lkdtm_parse_commandline() == -EINVAL) {
- pr_info("Invalid command\n");
- goto out_err;
- }
-
- if (lkdtm_crashpoint != CN_INVALID && lkdtm_crashtype != CT_NONE) {
- ret = lkdtm_register_cpoint(lkdtm_crashpoint);
+ /* Install crashpoint if one was selected. */
+ if (crashpoint) {
+ ret = lkdtm_register_cpoint(crashpoint, crashtype);
if (ret < 0) {
- pr_info("Invalid crash point %d\n", lkdtm_crashpoint);
+ pr_info("Invalid crashpoint %s\n", crashpoint->name);
goto out_err;
}
pr_info("Crash point %s of type %s registered\n",
- cpoint_name, cpoint_type);
+ crashpoint->name, cpoint_type);
} else {
pr_info("No crash points registered, enable through debugfs\n");
}
@@ -789,7 +531,7 @@ static void __exit lkdtm_module_exit(void)
/* Handle test-specific clean-up. */
lkdtm_usercopy_exit();
- unregister_jprobe(&lkdtm_jprobe);
+ unregister_jprobe(lkdtm_jprobe);
pr_info("Crash point unregistered\n");
}
@@ -797,4 +539,4 @@ module_init(lkdtm_module_init);
module_exit(lkdtm_module_exit);
MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Kprobe module for testing crash dumps");
+MODULE_DESCRIPTION("Kernel crash testing module");
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH] lkdtm: hide unused functions
2016-07-06 22:33 ` [PATCH 12/12] lkdtm: use struct arrays instead of enums Kees Cook
@ 2016-07-15 11:35 ` Arnd Bergmann
2016-07-15 22:54 ` Kees Cook
0 siblings, 1 reply; 15+ messages in thread
From: Arnd Bergmann @ 2016-07-15 11:35 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-kernel, Greg Kroah-Hartman
A conversion of the lkdtm core module added an "#ifdef CONFIG_KPROBES" check,
but a number of functions then become unused:
drivers/misc/lkdtm_core.c:340:16: error: 'lkdtm_debugfs_entry' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:122:12: error: 'jp_generic_ide_ioctl' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:114:12: error: 'jp_scsi_dispatch_cmd' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:106:12: error: 'jp_hrtimer_start' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:97:22: error: 'jp_shrink_inactive_list' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:89:13: error: 'jp_ll_rw_block' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:83:13: error: 'jp_tasklet_action' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:75:20: error: 'jp_handle_irq_event' defined but not used [-Werror=unused-function]
drivers/misc/lkdtm_core.c:68:21: error: 'jp_do_irq' defined but not used [-Werror=unused-function]
This adds the same #ifdef everywhere. There is probably a better way to do the
same thing, but for now this avoids the new warnings.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: c479e3fd8870 ("lkdtm: use struct arrays instead of enums")
---
On Wednesday, July 6, 2016 3:33:31 PM CEST Kees Cook wrote:
> +
> +/* Define the possible places where we can trigger a crash point. */
> +struct crashpoint crashpoints[] = {
> + CRASHPOINT("DIRECT", direct_entry,
> + NULL, NULL),
> +#ifdef CONFIG_KPROBES
> + CRASHPOINT("INT_HARDWARE_ENTRY", lkdtm_debugfs_entry,
> + "do_IRQ", jp_do_irq),
> + CRASHPOINT("INT_HW_IRQ_EN", lkdtm_debugfs_entry,
> + "handle_IRQ_event", jp_handle_irq_event),
> + CRASHPOINT("INT_TASKLET_ENTRY", lkdtm_debugfs_entry,
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index de29a339242a..6bdef16f95a2 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -53,12 +53,14 @@
#define DEFAULT_COUNT 10
-static void lkdtm_handler(void);
static int lkdtm_debugfs_open(struct inode *inode, struct file *file);
static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
size_t count, loff_t *off);
static ssize_t direct_entry(struct file *f, const char __user *user_buf,
size_t count, loff_t *off);
+
+#ifdef CONFIG_KPROBES
+static void lkdtm_handler(void);
static ssize_t lkdtm_debugfs_entry(struct file *f,
const char __user *user_buf,
size_t count, loff_t *off);
@@ -128,7 +130,7 @@ static int jp_generic_ide_ioctl(ide_drive_t *drive, struct file *file,
return 0;
}
#endif
-
+#endif
/* Crash points */
struct crashpoint {
@@ -240,7 +242,9 @@ struct crashtype *lkdtm_crashtype;
/* Global crash counter and spinlock. */
static int crash_count = DEFAULT_COUNT;
+#ifdef CONFIG_KPROBES
static DEFINE_SPINLOCK(crash_count_lock);
+#endif
/* Module parameters */
static int recur_count = -1;
@@ -285,6 +289,7 @@ static noinline void lkdtm_do_action(struct crashtype *crashtype)
crashtype->func();
}
+#ifdef CONFIG_KPROBES
/* Called by jprobe entry points. */
static void lkdtm_handler(void)
{
@@ -307,6 +312,7 @@ static void lkdtm_handler(void)
if (do_it)
lkdtm_do_action(lkdtm_crashtype);
}
+#endif
static int lkdtm_register_cpoint(struct crashpoint *crashpoint,
struct crashtype *crashtype)
@@ -337,6 +343,7 @@ static int lkdtm_register_cpoint(struct crashpoint *crashpoint,
return ret;
}
+#ifdef CONFIG_KPROBES
static ssize_t lkdtm_debugfs_entry(struct file *f,
const char __user *user_buf,
size_t count, loff_t *off)
@@ -374,6 +381,7 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
return count;
}
+#endif
/* Generic read callback that just prints out the available crash types */
static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] lkdtm: hide unused functions
2016-07-15 11:35 ` [PATCH] lkdtm: hide unused functions Arnd Bergmann
@ 2016-07-15 22:54 ` Kees Cook
0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2016-07-15 22:54 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: LKML, Greg Kroah-Hartman
On Fri, Jul 15, 2016 at 4:35 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> A conversion of the lkdtm core module added an "#ifdef CONFIG_KPROBES" check,
> but a number of functions then become unused:
>
> drivers/misc/lkdtm_core.c:340:16: error: 'lkdtm_debugfs_entry' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:122:12: error: 'jp_generic_ide_ioctl' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:114:12: error: 'jp_scsi_dispatch_cmd' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:106:12: error: 'jp_hrtimer_start' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:97:22: error: 'jp_shrink_inactive_list' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:89:13: error: 'jp_ll_rw_block' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:83:13: error: 'jp_tasklet_action' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:75:20: error: 'jp_handle_irq_event' defined but not used [-Werror=unused-function]
> drivers/misc/lkdtm_core.c:68:21: error: 'jp_do_irq' defined but not used [-Werror=unused-function]
>
> This adds the same #ifdef everywhere. There is probably a better way to do the
> same thing, but for now this avoids the new warnings.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: c479e3fd8870 ("lkdtm: use struct arrays instead of enums")
Ah! Thank you very much. I'm going to make a small change and get this
into my tree.
-Kees
> ---
>
> On Wednesday, July 6, 2016 3:33:31 PM CEST Kees Cook wrote:
>> +
>> +/* Define the possible places where we can trigger a crash point. */
>> +struct crashpoint crashpoints[] = {
>> + CRASHPOINT("DIRECT", direct_entry,
>> + NULL, NULL),
>> +#ifdef CONFIG_KPROBES
>> + CRASHPOINT("INT_HARDWARE_ENTRY", lkdtm_debugfs_entry,
>> + "do_IRQ", jp_do_irq),
>> + CRASHPOINT("INT_HW_IRQ_EN", lkdtm_debugfs_entry,
>> + "handle_IRQ_event", jp_handle_irq_event),
>> + CRASHPOINT("INT_TASKLET_ENTRY", lkdtm_debugfs_entry,
>
> diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
> index de29a339242a..6bdef16f95a2 100644
> --- a/drivers/misc/lkdtm_core.c
> +++ b/drivers/misc/lkdtm_core.c
> @@ -53,12 +53,14 @@
>
> #define DEFAULT_COUNT 10
>
> -static void lkdtm_handler(void);
> static int lkdtm_debugfs_open(struct inode *inode, struct file *file);
> static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
> size_t count, loff_t *off);
> static ssize_t direct_entry(struct file *f, const char __user *user_buf,
> size_t count, loff_t *off);
> +
> +#ifdef CONFIG_KPROBES
> +static void lkdtm_handler(void);
> static ssize_t lkdtm_debugfs_entry(struct file *f,
> const char __user *user_buf,
> size_t count, loff_t *off);
> @@ -128,7 +130,7 @@ static int jp_generic_ide_ioctl(ide_drive_t *drive, struct file *file,
> return 0;
> }
> #endif
> -
> +#endif
>
> /* Crash points */
> struct crashpoint {
> @@ -240,7 +242,9 @@ struct crashtype *lkdtm_crashtype;
>
> /* Global crash counter and spinlock. */
> static int crash_count = DEFAULT_COUNT;
> +#ifdef CONFIG_KPROBES
> static DEFINE_SPINLOCK(crash_count_lock);
> +#endif
>
> /* Module parameters */
> static int recur_count = -1;
> @@ -285,6 +289,7 @@ static noinline void lkdtm_do_action(struct crashtype *crashtype)
> crashtype->func();
> }
>
> +#ifdef CONFIG_KPROBES
> /* Called by jprobe entry points. */
> static void lkdtm_handler(void)
> {
> @@ -307,6 +312,7 @@ static void lkdtm_handler(void)
> if (do_it)
> lkdtm_do_action(lkdtm_crashtype);
> }
> +#endif
>
> static int lkdtm_register_cpoint(struct crashpoint *crashpoint,
> struct crashtype *crashtype)
> @@ -337,6 +343,7 @@ static int lkdtm_register_cpoint(struct crashpoint *crashpoint,
> return ret;
> }
>
> +#ifdef CONFIG_KPROBES
> static ssize_t lkdtm_debugfs_entry(struct file *f,
> const char __user *user_buf,
> size_t count, loff_t *off)
> @@ -374,6 +381,7 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
>
> return count;
> }
> +#endif
>
> /* Generic read callback that just prints out the available crash types */
> static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
>
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 15+ messages in thread