[PATCH 0/2] MIPS: Minor EVA related unwind fixes

[PATCH 0/2] MIPS: Minor EVA related unwind fixes

[James Hogan]

Here are a couple of minor EVA related stack unwind fixes, which
prevent the use of unwind_stack() (which unwinds using kallsyms
knowledge of where kernel functions begin and end) to unwind userland
code, particularly where the PC is a valid kernel address, which could
happen innocently with EVA due to the overlapped user/kernel address
spaces.

Note that unwind_stack() is defensive enough that it shouldn't do any
harm, so I've only tagged stable back to v3.15, when EVA was added and
it could conceivably happen under normal operation.

A rough audit shows no other uses of __kernel_text_address() in relation
to possibly user mode register contexts.


The first patch fixes unwind_stack() itself not to unwind through an
exception frame if it represents the user register context, preventing
itself being called with that context next.

The second patch fixes show_backtrace() to fall back to raw backtrace
for user mode register contexts, rather than using unwind_stack(). This
is used on certain faults which can be triggered by userland (namely
unaligned accesses when reporting is enabled in debugfs).


James Hogan (2):
  MIPS: Don't unwind to user mode with EVA
  MIPS: Avoid using unwind_stack() with usermode

 arch/mips/kernel/process.c | 2 +-
 arch/mips/kernel/traps.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 3.15+
-- 
2.4.10

[PATCH 1/2] MIPS: Don't unwind to user mode with EVA

[James Hogan]

When unwinding through IRQs and exceptions, the unwinding only continues
if the PC is a kernel text address, however since EVA it is possible for
user and kernel address ranges to overlap, potentially allowing
unwinding to continue to user mode if the user PC happens to be in the
kernel text address range.

Adjust the check to also ensure that the register state from before the
exception is actually running in kernel mode, i.e. !user_mode(regs).

I don't believe any harm can come of this problem, since the PC is only
output, the stack pointer is checked to ensure it resides within the
task's stack page before it is dereferenced in search of the return
address, and the return address register is similarly only output (if
the PC is in a leaf function or the beginning of a non-leaf function).

However unwind_stack() is only meant for unwinding kernel code, so to be
correct the unwind should stop there.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 3.15+
---
 arch/mips/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index f2975d4d1e44..6b3ae73cda3f 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -457,7 +457,7 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
 		    *sp + sizeof(*regs) <= stack_page + THREAD_SIZE - 32) {
 			regs = (struct pt_regs *)*sp;
 			pc = regs->cp0_epc;
-			if (__kernel_text_address(pc)) {
+			if (!user_mode(regs) && __kernel_text_address(pc)) {
 				*sp = regs->regs[29];
 				*ra = regs->regs[31];
 				return pc;
-- 
2.4.10

Re: [PATCH 1/2] MIPS: Don't unwind to user mode with EVA

[Leonid Yegoshin]

OK.

Re: [PATCH 1/2] MIPS: Don't unwind to user mode with EVA

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 137 bytes --]

On Fri, Dec 04, 2015 at 04:21:09PM -0800, Leonid Yegoshin wrote:
> OK.

Thanks Leonid. Can that be taken as a Reviewed-by?

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] MIPS: Don't unwind to user mode with EVA

[Leonid Yegoshin]

On 12/07/2015 12:43 AM, James Hogan wrote:
> On Fri, Dec 04, 2015 at 04:21:09PM -0800, Leonid Yegoshin wrote:
>> OK.
> Thanks Leonid. Can that be taken as a Reviewed-by?
>
> Cheers
> James
Yes, please.

- Leonid

[PATCH 2/2] MIPS: Avoid using unwind_stack() with usermode

[James Hogan]

When showing backtraces in response to traps, for example crashes and
address errors (usually unaligned accesses) when they are set in debugfs
to be reported, unwind_stack will be used if the PC was in the kernel
text address range. However since EVA it is possible for user and kernel
address ranges to overlap, and even without EVA userland can still
trigger an address error by jumping to a KSeg0 address.

Adjust the check to also ensure that it was running in kernel mode. I
don't believe any harm can come of this problem, since unwind_stack() is
sufficiently defensive, however it is only meant for unwinding kernel
code, so to be correct it should use the raw backtracing instead.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # 3.15+
Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 arch/mips/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 886cb1976e90..52ef4eb41f43 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -144,7 +144,7 @@ static void show_backtrace(struct task_struct *task, const struct pt_regs *regs)
 	if (!task)
 		task = current;
 
-	if (raw_show_trace || !__kernel_text_address(pc)) {
+	if (raw_show_trace || user_mode(regs) || !__kernel_text_address(pc)) {
 		show_raw_backtrace(sp);
 		return;
 	}
-- 
2.4.10

Re: [PATCH 2/2] MIPS: Avoid using unwind_stack() with usermode

[Leonid Yegoshin]

OK.