[PATCH AUTOSEL 5.10 20/31] um: allocate a guard page to helper threads

From: Sasha Levin
Date: Wed Dec 30 2020 - 08:10:57 EST


From: Johannes Berg <johannes.berg@xxxxxxxxx>

[ Upstream commit ef4459a6da0955b533ebfc97a7d756ac090f50c9 ]

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@xxxxxxxxx>
Signed-off-by: Richard Weinberger <richard@xxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
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(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index b12c1b0d3e1d0..e337702c30c78 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1195,7 +1195,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 ccafb62e8ccec..c3ccbc2bbd2ce 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 9505a7e87396a..bb352dc446870 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -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)
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index 9fa6e4187d4fb..feb48d796e005 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;

--
2.27.0