um: allocate a guard page to helper threads
authorJohannes Berg <johannes.berg@intel.com>
Sat, 5 Dec 2020 20:50:18 +0000 (21:50 +0100)
committerRichard Weinberger <richard@nod.at>
Sun, 13 Dec 2020 21:38:06 +0000 (22:38 +0100)
We've been running into stack overflows in helper threads
corrupting memory (e.g. because somebody put printf() or
os_info() there), so to avoid those causing hard-to-debug
issues later on, allocate a guard page for helper thread
stacks and mark it read-only.

Unfortunately, the crash dump at that point is useless as
the stack tracer will try to backtrace the *kernel* thread,
not the helper thread, but at least we don't survive to a
random issue caused by corruption.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
arch/um/drivers/ubd_kern.c
arch/um/include/shared/kern_util.h
arch/um/kernel/process.c
arch/um/os-Linux/helper.c

index 7e07b32..13b1fe6 100644 (file)
@@ -1241,7 +1241,7 @@ static int __init ubd_driver_init(void){
                /* Letting ubd=sync be like using ubd#s= instead of ubd#= is
                 * enough. So use anyway the io thread. */
        }
-       stack = alloc_stack(0, 0);
+       stack = alloc_stack(0);
        io_pid = start_io_thread(stack + PAGE_SIZE - sizeof(void *),
                                 &thread_fd);
        if(io_pid < 0){
index 2888ec8..d8c279e 100644 (file)
@@ -19,7 +19,7 @@ extern int kmalloc_ok;
 #define UML_ROUND_UP(addr) \
        ((((unsigned long) addr) + PAGE_SIZE - 1) & PAGE_MASK)
 
-extern unsigned long alloc_stack(int order, int atomic);
+extern unsigned long alloc_stack(int atomic);
 extern void free_stack(unsigned long stack, int order);
 
 struct pt_regs;
index 81d508d..2a986ec 100644 (file)
@@ -32,6 +32,7 @@
 #include <os.h>
 #include <skas.h>
 #include <linux/time-internal.h>
+#include <asm/set_memory.h>
 
 /*
  * This is a per-cpu array.  A processor only modifies its entry and it only
@@ -62,16 +63,18 @@ void free_stack(unsigned long stack, int order)
        free_pages(stack, order);
 }
 
-unsigned long alloc_stack(int order, int atomic)
+unsigned long alloc_stack(int atomic)
 {
-       unsigned long page;
+       unsigned long addr;
        gfp_t flags = GFP_KERNEL;
 
        if (atomic)
                flags = GFP_ATOMIC;
-       page = __get_free_pages(flags, order);
+       addr = __get_free_pages(flags, 1);
 
-       return page;
+       set_memory_ro(addr, 1);
+
+       return addr + PAGE_SIZE;
 }
 
 static inline void set_current(struct task_struct *task)
index 9fa6e41..feb48d7 100644 (file)
@@ -45,7 +45,7 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv)
        unsigned long stack, sp;
        int pid, fds[2], ret, n;
 
-       stack = alloc_stack(0, __cant_sleep());
+       stack = alloc_stack(__cant_sleep());
        if (stack == 0)
                return -ENOMEM;
 
@@ -116,7 +116,7 @@ int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags,
        unsigned long stack, sp;
        int pid, status, err;
 
-       stack = alloc_stack(0, __cant_sleep());
+       stack = alloc_stack(__cant_sleep());
        if (stack == 0)
                return -ENOMEM;