[PATCH v2 0/2] Let illegal access to user-space memory die

[PATCH v2 0/2] Let illegal access to user-space memory die

[Eric Lin]

Accesses to user-space memory without calling uaccess routine
leads to hanging in page fault handler. Like arm64, we let it
die earlier in page fault handler.

Changes in v2:
    -Add a die_kernel_fault() helper
    -Split one long line code into two

Eric Lin (2):
  riscv/mm: Introduce a die_kernel_fault() helper function
  riscv/mm: Prevent kernel module to access user memory without uaccess
    routines

 arch/riscv/mm/fault.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.17.0

[PATCH v2 1/2] riscv/mm: Introduce a die_kernel_fault() helper function

[Eric Lin]

Like arm64, this patch adds a die_kernel_fault() helper
to ensure the same semantics for the different kernel faults.

Signed-off-by: Eric Lin <tesheng@andestech.com>
Cc: Alan Kao <alankao@andestech.com>
---
 arch/riscv/mm/fault.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 3c8b9e433c67..0bcfd0e1b39e 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -19,6 +19,19 @@
 
 #include "../kernel/head.h"
 
+static void die_kernel_fault(const char *msg, unsigned long addr,
+		struct pt_regs *regs)
+{
+	bust_spinlocks(1);
+
+	pr_alert("Unable to handle kernel %s at virtual address " REG_FMT "\n", msg,
+			addr);
+
+	bust_spinlocks(0);
+	die(regs, "Oops");
+	do_exit(SIGKILL);
+}
+
 static inline void no_context(struct pt_regs *regs, unsigned long addr)
 {
 	/* Are we prepared to handle this kernel fault? */
-- 
2.17.0

[PATCH v2 2/2] riscv/mm: Prevent kernel module to access user memory without uaccess routines

[Eric Lin]

We found this issue in an legacy out-of-tree kernel module
which didn't properly access user space pointer by get/put_user().
Such an illegal access loops in the page fault handler.
To resolve this, let it die here.

Signed-off-by: Eric Lin <tesheng@andestech.com>
Cc: Alan Kao <alankao@andestech.com>
---
 arch/riscv/mm/fault.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 0bcfd0e1b39e..00884c1bac28 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -245,6 +245,11 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	if (user_mode(regs))
 		flags |= FAULT_FLAG_USER;
 
+	if (!user_mode(regs) && addr < TASK_SIZE &&
+			unlikely(!(regs->status & SR_SUM)))
+		die_kernel_fault("access to user memory without uaccess routines",
+				addr, regs);
+
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
 	if (cause == EXC_STORE_PAGE_FAULT)
-- 
2.17.0

Re: [PATCH v2 0/2] Let illegal access to user-space memory die

[Pekka Enberg]

Hi Eric,

On Thu, Dec 3, 2020 at 8:51 AM Eric Lin <tesheng@andestech.com> wrote:
>
> Accesses to user-space memory without calling uaccess routine
> leads to hanging in page fault handler. Like arm64, we let it
> die earlier in page fault handler.
>
> Changes in v2:
>     -Add a die_kernel_fault() helper
>     -Split one long line code into two

Please also make no_context() use the new helper. Other than that:

Reviewed-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH v2 0/2] Let illegal access to user-space memory die

[Eric Lin]

On Thu, Dec 03, 2020 at 03:29:57PM +0800, Pekka Enberg wrote:

Hi Pekka, 

> Hi Eric,
> 
> On Thu, Dec 3, 2020 at 8:51 AM Eric Lin <tesheng@andestech.com> wrote:
> >
> > Accesses to user-space memory without calling uaccess routine
> > leads to hanging in page fault handler. Like arm64, we let it
> > die earlier in page fault handler.
> >
> > Changes in v2:
> >     -Add a die_kernel_fault() helper
> >     -Split one long line code into two
> 
> Please also make no_context() use the new helper. Other than that:
> 

OK, I'll make no_context() use the new helper in v3.
Thanks for your review.

> Reviewed-by: Pekka Enberg <penberg@kernel.org>