From ef4459a6da0955b533ebfc97a7d756ac090f50c9 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Sat, 5 Dec 2020 21:50:18 +0100 Subject: um: allocate a guard page to helper threads 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 Signed-off-by: Richard Weinberger --- arch/um/drivers/ubd_kern.c | 2 +- arch/um/include/shared/kern_util.h | 2 +- arch/um/kernel/process.c | 11 +++++++---- arch/um/os-Linux/helper.c | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) (limited to 'arch/um') diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 7e07b321e847..13b1fe694b90 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -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){ diff --git a/arch/um/include/shared/kern_util.h b/arch/um/include/shared/kern_util.h index 2888ec812f6e..d8c279e3312f 100644 --- a/arch/um/include/shared/kern_util.h +++ b/arch/um/include/shared/kern_util.h @@ -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; diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c index 81d508daf67c..2a986ece5478 100644 --- a/arch/um/kernel/process.c +++ b/arch/um/kernel/process.c @@ -32,6 +32,7 @@ #include #include #include +#include /* * 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) diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c index 9fa6e4187d4f..feb48d796e00 100644 --- a/arch/um/os-Linux/helper.c +++ b/arch/um/os-Linux/helper.c @@ -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; -- cgit v1.2.3