| Message ID | 1480652545-4457-1-git-send-email-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 1, 2016 at 8:22 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > This adds two tests, to check that a read or write to LIST_POISON1 and > ZERO_SIZE_PTR are blocked. > > The default values for both (256 and 16) typically fall in the range > of valid user space addresses. However in general mmap_min_addr is 64K, > which prevents user space from mapping anything at those addresses. > > However it's feasible that an attacker will be able to find a way to > cause an access at an offset from either value, and if that offset is > greater than 64K then they can access user space again. > > To simulate that case, in the test we create a user mapping at > approximately mmap_min_addr, and offset the pointer by that amount. This > gives the test the greatest chance of failing (ie. an access > succeeding). We don't actually use mmap_min_addr, because that would > require exporting it to modules, instead we use the default value at > compile time as a reasonable approximation. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Thanks for this! I like it. :) Acked-by: Kees Cook <keescook@chromium.org> Greg, can you take this into your tree? -Kees > --- > drivers/misc/lkdtm.h | 2 ++ > drivers/misc/lkdtm_bugs.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ > drivers/misc/lkdtm_core.c | 2 ++ > 3 files changed, 52 insertions(+) > > v2: Fix 32-bit compile errors by using int not long. > Avoid need to export mmap_min_addr by using CONFIG_DEFAULT_MMAP_MIN_ADDR. > > diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h > index fdf954c2107f..cc207f7824f9 100644 > --- a/drivers/misc/lkdtm.h > +++ b/drivers/misc/lkdtm.h > @@ -21,6 +21,8 @@ void lkdtm_SPINLOCKUP(void); > void lkdtm_HUNG_TASK(void); > void lkdtm_ATOMIC_UNDERFLOW(void); > void lkdtm_ATOMIC_OVERFLOW(void); > +void lkdtm_ACCESS_LIST_POISON(void); > +void lkdtm_ACCESS_ZERO_SIZE_PTR(void); > > /* lkdtm_heap.c */ > void lkdtm_OVERWRITE_ALLOCATION(void); > diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c > index 30e62dd7e7ca..9ae079ac9a93 100644 > --- a/drivers/misc/lkdtm_bugs.c > +++ b/drivers/misc/lkdtm_bugs.c > @@ -5,7 +5,10 @@ > * test source files. > */ > #include "lkdtm.h" > +#include <linux/mman.h> > #include <linux/sched.h> > +#include <linux/security.h> > +#include <linux/slab.h> > > /* > * Make sure our attempts to over run the kernel stack doesn't trigger > @@ -147,3 +150,48 @@ void lkdtm_ATOMIC_OVERFLOW(void) > pr_info("attempting bad atomic overflow\n"); > atomic_inc(&over); > } > + > +static void test_poison_ptr(void *base, const char *desc) > +{ > + unsigned int *ptr, bias, val; > + unsigned long uaddr; > + > + /* We'd rather not export mmap_min_addr, so use the default instead */ > + bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR); > + > + uaddr = vm_mmap(NULL, bias, PAGE_SIZE, PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, 0); > + if (uaddr >= TASK_SIZE) { > + pr_warn("Failed to allocate user memory, can't perform test.\n"); > + return; > + } > + > + /* > + * Creating a mapping and adding mmap_min_addr to the value is cheating > + * in a way. But it simulates the case where an attacker is able to > + * cause an access at a small offset from the base value, leading to a > + * user space access. If an arch doesn't define CONFIG_ILLEGAL_POINTER_VALUE > + * then it's likely this will work in the absence of other protections. > + */ > + ptr = bias + base; > + > + pr_info("attempting read of %s %p\n", desc, ptr); > + val = *ptr; > + pr_info("FAIL: Was able to read %s! Got 0x%x\n", desc, val); > + > + pr_info("attempting write of %s %p\n", desc, ptr); > + *ptr = 0xdeadbeefU; > + pr_info("FAIL: Was able to write %s! Now = 0x%x\n", desc, *ptr); > + > + vm_munmap(uaddr, PAGE_SIZE); > +} > + > +void lkdtm_ACCESS_LIST_POISON(void) > +{ > + test_poison_ptr(LIST_POISON1, "LIST_POISON"); > +} > + > +void lkdtm_ACCESS_ZERO_SIZE_PTR(void) > +{ > + test_poison_ptr(ZERO_SIZE_PTR, "ZERO_SIZE_PTR"); > +} > diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c > index f9154b8d67f6..025a0ee8d8ee 100644 > --- a/drivers/misc/lkdtm_core.c > +++ b/drivers/misc/lkdtm_core.c > @@ -220,6 +220,8 @@ struct crashtype crashtypes[] = { > CRASHTYPE(WRITE_KERN), > CRASHTYPE(ATOMIC_UNDERFLOW), > CRASHTYPE(ATOMIC_OVERFLOW), > + CRASHTYPE(ACCESS_LIST_POISON), > + CRASHTYPE(ACCESS_ZERO_SIZE_PTR), > CRASHTYPE(USERCOPY_HEAP_SIZE_TO), > CRASHTYPE(USERCOPY_HEAP_SIZE_FROM), > CRASHTYPE(USERCOPY_HEAP_FLAG_TO), > -- > 2.7.4 >
Hi Michael,
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.9-rc7]
[cannot apply to next-20161202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-Ellerman/lkdtm-Add-tests-for-LIST_POISON-and-ZERO_SIZE_PTR/20161203-124958
config: blackfin-allyesconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All error/warnings (new ones prefixed by >>):
In file included from include/linux/cache.h:4:0,
from include/linux/printk.h:8,
from include/linux/kernel.h:13,
from drivers/misc/lkdtm.h:6,
from drivers/misc/lkdtm_bugs.c:7:
drivers/misc/lkdtm_bugs.c: In function 'test_poison_ptr':
>> drivers/misc/lkdtm_bugs.c:160:20: error: 'CONFIG_DEFAULT_MMAP_MIN_ADDR' undeclared (first use in this function)
bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR);
^
include/uapi/linux/kernel.h:10:41: note: in definition of macro '__ALIGN_KERNEL_MASK'
#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
^
>> include/linux/kernel.h:48:22: note: in expansion of macro '__ALIGN_KERNEL'
#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
^~~~~~~~~~~~~~
>> include/linux/mm.h:126:26: note: in expansion of macro 'ALIGN'
#define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE)
^~~~~
>> drivers/misc/lkdtm_bugs.c:160:9: note: in expansion of macro 'PAGE_ALIGN'
bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR);
^~~~~~~~~~
drivers/misc/lkdtm_bugs.c:160:20: note: each undeclared identifier is reported only once for each function it appears in
bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR);
^
include/uapi/linux/kernel.h:10:41: note: in definition of macro '__ALIGN_KERNEL_MASK'
#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
^
>> include/linux/kernel.h:48:22: note: in expansion of macro '__ALIGN_KERNEL'
#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
^~~~~~~~~~~~~~
>> include/linux/mm.h:126:26: note: in expansion of macro 'ALIGN'
#define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE)
^~~~~
>> drivers/misc/lkdtm_bugs.c:160:9: note: in expansion of macro 'PAGE_ALIGN'
bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR);
^~~~~~~~~~
vim +/CONFIG_DEFAULT_MMAP_MIN_ADDR +160 drivers/misc/lkdtm_bugs.c
1 /*
2 * This is for all the tests related to logic bugs (e.g. bad dereferences,
3 * bad alignment, bad loops, bad locking, bad scheduling, deep stacks, and
4 * lockups) along with other things that don't fit well into existing LKDTM
5 * test source files.
6 */
> 7 #include "lkdtm.h"
8 #include <linux/mman.h>
9 #include <linux/sched.h>
10 #include <linux/security.h>
11 #include <linux/slab.h>
12
13 /*
14 * Make sure our attempts to over run the kernel stack doesn't trigger
15 * a compiler warning when CONFIG_FRAME_WARN is set. Then make sure we
16 * recurse past the end of THREAD_SIZE by default.
17 */
18 #if defined(CONFIG_FRAME_WARN) && (CONFIG_FRAME_WARN > 0)
19 #define REC_STACK_SIZE (CONFIG_FRAME_WARN / 2)
20 #else
21 #define REC_STACK_SIZE (THREAD_SIZE / 8)
22 #endif
23 #define REC_NUM_DEFAULT ((THREAD_SIZE / REC_STACK_SIZE) * 2)
24
25 static int recur_count = REC_NUM_DEFAULT;
26
27 static DEFINE_SPINLOCK(lock_me_up);
28
29 static int recursive_loop(int remaining)
30 {
31 char buf[REC_STACK_SIZE];
32
33 /* Make sure compiler does not optimize this away. */
34 memset(buf, (remaining & 0xff) | 0x1, REC_STACK_SIZE);
35 if (!remaining)
36 return 0;
37 else
38 return recursive_loop(remaining - 1);
39 }
40
41 /* If the depth is negative, use the default, otherwise keep parameter. */
42 void __init lkdtm_bugs_init(int *recur_param)
43 {
44 if (*recur_param < 0)
45 *recur_param = recur_count;
46 else
47 recur_count = *recur_param;
48 }
49
50 void lkdtm_PANIC(void)
51 {
52 panic("dumptest");
53 }
54
55 void lkdtm_BUG(void)
56 {
57 BUG();
58 }
59
60 void lkdtm_WARNING(void)
61 {
62 WARN_ON(1);
63 }
64
65 void lkdtm_EXCEPTION(void)
66 {
67 *((int *) 0) = 0;
68 }
69
70 void lkdtm_LOOP(void)
71 {
72 for (;;)
73 ;
74 }
75
76 void lkdtm_OVERFLOW(void)
77 {
78 (void) recursive_loop(recur_count);
79 }
80
81 noinline void lkdtm_CORRUPT_STACK(void)
82 {
83 /* Use default char array length that triggers stack protection. */
84 char data[8];
85
86 memset((void *)data, 'a', 64);
87 pr_info("Corrupted stack with '%16s'...\n", data);
88 }
89
90 void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void)
91 {
92 static u8 data[5] __attribute__((aligned(4))) = {1, 2, 3, 4, 5};
93 u32 *p;
94 u32 val = 0x12345678;
95
96 p = (u32 *)(data + 1);
97 if (*p == 0)
98 val = 0x87654321;
99 *p = val;
100 }
101
102 void lkdtm_SOFTLOCKUP(void)
103 {
104 preempt_disable();
105 for (;;)
106 cpu_relax();
107 }
108
109 void lkdtm_HARDLOCKUP(void)
110 {
111 local_irq_disable();
112 for (;;)
113 cpu_relax();
114 }
115
116 void lkdtm_SPINLOCKUP(void)
117 {
118 /* Must be called twice to trigger. */
119 spin_lock(&lock_me_up);
120 /* Let sparse know we intended to exit holding the lock. */
121 __release(&lock_me_up);
122 }
123
124 void lkdtm_HUNG_TASK(void)
125 {
126 set_current_state(TASK_UNINTERRUPTIBLE);
127 schedule();
128 }
129
130 void lkdtm_ATOMIC_UNDERFLOW(void)
131 {
132 atomic_t under = ATOMIC_INIT(INT_MIN);
133
134 pr_info("attempting good atomic increment\n");
135 atomic_inc(&under);
136 atomic_dec(&under);
137
138 pr_info("attempting bad atomic underflow\n");
139 atomic_dec(&under);
140 }
141
142 void lkdtm_ATOMIC_OVERFLOW(void)
143 {
144 atomic_t over = ATOMIC_INIT(INT_MAX);
145
146 pr_info("attempting good atomic decrement\n");
147 atomic_dec(&over);
148 atomic_inc(&over);
149
150 pr_info("attempting bad atomic overflow\n");
151 atomic_inc(&over);
152 }
153
154 static void test_poison_ptr(void *base, const char *desc)
155 {
156 unsigned int *ptr, bias, val;
157 unsigned long uaddr;
158
159 /* We'd rather not export mmap_min_addr, so use the default instead */
> 160 bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR);
161
162 uaddr = vm_mmap(NULL, bias, PAGE_SIZE, PROT_READ | PROT_WRITE,
163 MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, 0);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Dec 02, 2016 at 01:32:24PM -0800, Kees Cook wrote: > On Thu, Dec 1, 2016 at 8:22 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > > This adds two tests, to check that a read or write to LIST_POISON1 and > > ZERO_SIZE_PTR are blocked. > > > > The default values for both (256 and 16) typically fall in the range > > of valid user space addresses. However in general mmap_min_addr is 64K, > > which prevents user space from mapping anything at those addresses. > > > > However it's feasible that an attacker will be able to find a way to > > cause an access at an offset from either value, and if that offset is > > greater than 64K then they can access user space again. > > > > To simulate that case, in the test we create a user mapping at > > approximately mmap_min_addr, and offset the pointer by that amount. This > > gives the test the greatest chance of failing (ie. an access > > succeeding). We don't actually use mmap_min_addr, because that would > > require exporting it to modules, instead we use the default value at > > compile time as a reasonable approximation. > > > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > Thanks for this! I like it. :) > > Acked-by: Kees Cook <keescook@chromium.org> > > Greg, can you take this into your tree? Sure, will do so on Monday. thanks, greg k-h
On Sat, Dec 03, 2016 at 10:10:57AM +0100, Greg KH wrote: > On Fri, Dec 02, 2016 at 01:32:24PM -0800, Kees Cook wrote: > > On Thu, Dec 1, 2016 at 8:22 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > > > This adds two tests, to check that a read or write to LIST_POISON1 and > > > ZERO_SIZE_PTR are blocked. > > > > > > The default values for both (256 and 16) typically fall in the range > > > of valid user space addresses. However in general mmap_min_addr is 64K, > > > which prevents user space from mapping anything at those addresses. > > > > > > However it's feasible that an attacker will be able to find a way to > > > cause an access at an offset from either value, and if that offset is > > > greater than 64K then they can access user space again. > > > > > > To simulate that case, in the test we create a user mapping at > > > approximately mmap_min_addr, and offset the pointer by that amount. This > > > gives the test the greatest chance of failing (ie. an access > > > succeeding). We don't actually use mmap_min_addr, because that would > > > require exporting it to modules, instead we use the default value at > > > compile time as a reasonable approximation. > > > > > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > > > Thanks for this! I like it. :) > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > Greg, can you take this into your tree? > > Sure, will do so on Monday. Oops, no, I will not, kbuild reports build errors with it :(
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h index fdf954c2107f..cc207f7824f9 100644 --- a/drivers/misc/lkdtm.h +++ b/drivers/misc/lkdtm.h @@ -21,6 +21,8 @@ void lkdtm_SPINLOCKUP(void); void lkdtm_HUNG_TASK(void); void lkdtm_ATOMIC_UNDERFLOW(void); void lkdtm_ATOMIC_OVERFLOW(void); +void lkdtm_ACCESS_LIST_POISON(void); +void lkdtm_ACCESS_ZERO_SIZE_PTR(void); /* lkdtm_heap.c */ void lkdtm_OVERWRITE_ALLOCATION(void); diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c index 30e62dd7e7ca..9ae079ac9a93 100644 --- a/drivers/misc/lkdtm_bugs.c +++ b/drivers/misc/lkdtm_bugs.c @@ -5,7 +5,10 @@ * test source files. */ #include "lkdtm.h" +#include <linux/mman.h> #include <linux/sched.h> +#include <linux/security.h> +#include <linux/slab.h> /* * Make sure our attempts to over run the kernel stack doesn't trigger @@ -147,3 +150,48 @@ void lkdtm_ATOMIC_OVERFLOW(void) pr_info("attempting bad atomic overflow\n"); atomic_inc(&over); } + +static void test_poison_ptr(void *base, const char *desc) +{ + unsigned int *ptr, bias, val; + unsigned long uaddr; + + /* We'd rather not export mmap_min_addr, so use the default instead */ + bias = PAGE_ALIGN(CONFIG_DEFAULT_MMAP_MIN_ADDR); + + uaddr = vm_mmap(NULL, bias, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, 0); + if (uaddr >= TASK_SIZE) { + pr_warn("Failed to allocate user memory, can't perform test.\n"); + return; + } + + /* + * Creating a mapping and adding mmap_min_addr to the value is cheating + * in a way. But it simulates the case where an attacker is able to + * cause an access at a small offset from the base value, leading to a + * user space access. If an arch doesn't define CONFIG_ILLEGAL_POINTER_VALUE + * then it's likely this will work in the absence of other protections. + */ + ptr = bias + base; + + pr_info("attempting read of %s %p\n", desc, ptr); + val = *ptr; + pr_info("FAIL: Was able to read %s! Got 0x%x\n", desc, val); + + pr_info("attempting write of %s %p\n", desc, ptr); + *ptr = 0xdeadbeefU; + pr_info("FAIL: Was able to write %s! Now = 0x%x\n", desc, *ptr); + + vm_munmap(uaddr, PAGE_SIZE); +} + +void lkdtm_ACCESS_LIST_POISON(void) +{ + test_poison_ptr(LIST_POISON1, "LIST_POISON"); +} + +void lkdtm_ACCESS_ZERO_SIZE_PTR(void) +{ + test_poison_ptr(ZERO_SIZE_PTR, "ZERO_SIZE_PTR"); +} diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c index f9154b8d67f6..025a0ee8d8ee 100644 --- a/drivers/misc/lkdtm_core.c +++ b/drivers/misc/lkdtm_core.c @@ -220,6 +220,8 @@ struct crashtype crashtypes[] = { CRASHTYPE(WRITE_KERN), CRASHTYPE(ATOMIC_UNDERFLOW), CRASHTYPE(ATOMIC_OVERFLOW), + CRASHTYPE(ACCESS_LIST_POISON), + CRASHTYPE(ACCESS_ZERO_SIZE_PTR), CRASHTYPE(USERCOPY_HEAP_SIZE_TO), CRASHTYPE(USERCOPY_HEAP_SIZE_FROM), CRASHTYPE(USERCOPY_HEAP_FLAG_TO),
This adds two tests, to check that a read or write to LIST_POISON1 and ZERO_SIZE_PTR are blocked. The default values for both (256 and 16) typically fall in the range of valid user space addresses. However in general mmap_min_addr is 64K, which prevents user space from mapping anything at those addresses. However it's feasible that an attacker will be able to find a way to cause an access at an offset from either value, and if that offset is greater than 64K then they can access user space again. To simulate that case, in the test we create a user mapping at approximately mmap_min_addr, and offset the pointer by that amount. This gives the test the greatest chance of failing (ie. an access succeeding). We don't actually use mmap_min_addr, because that would require exporting it to modules, instead we use the default value at compile time as a reasonable approximation. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- drivers/misc/lkdtm.h | 2 ++ drivers/misc/lkdtm_bugs.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/misc/lkdtm_core.c | 2 ++ 3 files changed, 52 insertions(+) v2: Fix 32-bit compile errors by using int not long. Avoid need to export mmap_min_addr by using CONFIG_DEFAULT_MMAP_MIN_ADDR.