linux-kernel - [PATCH] RISC-V: Protect reads from other harts stack frames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed,  5 May 2021 23:13:52 -0700
From:   Palmer Dabbelt <palmer@...belt.com>
To:         linux-riscv@...ts.infradead.org
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>, aou@...s.berkeley.edu,
        wangkefeng.wang@...wei.com, akpm@...ux-foundation.org,
        0x7f454c46@...il.com, rostedt@...dmis.org, chenhuang5@...wei.com,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com, Palmer Dabbelt <palmerdabbelt@...gle.com>,
        syzbot+0806291048161061627c@...kaller.appspotmail.com,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: [PATCH] RISC-V: Protect reads from other harts stack frames

From: Palmer Dabbelt <palmerdabbelt@...gle.com>

The stack walking code wasn't correctly decorated with READ_ONCE_NOCHECK
when reading from other harts stack frames, which can trigger a kasan
failure.  This may also manifest as a bug, as without the READ_ONCE we
may get inconsistent results.

Reported-by: syzbot+0806291048161061627c@...kaller.appspotmail.com
Fixes: 5d8544e2d007 ("RISC-V: Generic library routines and assembly")
Suggested-by: Dmitry Vyukov <dvyukov@...gle.com>
Signed-off-by: Palmer Dabbelt <palmerdabbelt@...gle.com>

---

I don't actually have a test for stack walking aside from just crashing
the kernel and making sure things look roughly OK.  I haven't gotten
around to that because this got lost in the merge window shuffle, but I
thought I'd send this out in case someone has a better test for stack
walking so I can start running that.
---
 arch/riscv/kernel/stacktrace.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 3f893c9d9d85..7f3914756915 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -18,6 +18,9 @@ register const unsigned long sp_in_global __asm__("sp");
 
 #ifdef CONFIG_FRAME_POINTER
 
+#define READ_FRAME(frame, off)	\
+	(READ_ONCE_NOCHECK(*(unsigned long *)(frame + offsetof(struct stackframe, off))))
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			     bool (*fn)(void *, unsigned long), void *arg)
 {
@@ -40,7 +43,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 
 	for (;;) {
 		unsigned long low, high;
-		struct stackframe *frame;
+		unsigned long frame;
 
 		if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
 			break;
@@ -51,14 +54,14 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		if (unlikely(fp < low || fp > high || fp & 0x7))
 			break;
 		/* Unwind stack frame */
-		frame = (struct stackframe *)fp - 1;
+		frame = fp - sizeof(struct stackframe);
 		sp = fp;
-		if (regs && (regs->epc == pc) && (frame->fp & 0x7)) {
-			fp = frame->ra;
+		if (regs && (regs->epc == pc) && (READ_FRAME(frame, fp) & 0x7)) {
+			fp = READ_FRAME(frame, ra);
 			pc = regs->ra;
 		} else {
-			fp = frame->fp;
-			pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
+			fp = READ_FRAME(frame, fp);
+			pc = ftrace_graph_ret_addr(current, NULL, READ_FRAME(frame, ra),
 						   (unsigned long *)(fp - 8));
 		}
 
-- 
2.31.1.527.g47e6f16901-goog