* [PATCH v2 1/5] MIPS: Handle non word sized instructions when examining frame
2017-03-01 14:41 [PATCH v2 0/5] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
@ 2017-03-01 14:41 ` Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 2/5] MIPS: microMIPS: Fix decoding of addiusp instruction Matt Redfearn
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-01 14:41 UTC (permalink / raw)
To: Ralf Baechle
Cc: linux-mips, Matt Redfearn, Marcin Nowakowski, linux-kernel,
Paul Burton, Andrew Morton
Commit b6c7a324df37 ("MIPS: Fix get_frame_info() handling of microMIPS
function size") goes some way to fixing get_frame_info() to iterate over
microMIPS instuctions, but increments the instruction pointer using a
postincrement of the instruction pointer, which is of union
mips_instruction type. Since the union is sized to the largest member (a
word), but microMIPS instructions are a mix of halfword and word sizes,
the function does not always iterate correctly, ending up misaligned
with the instruction stream and interpreting it incorrectly.
Since the instruction modifying the stack pointer is usually the first
in the function, that one is usually handled correctly. But the
instruction which saves the return address to the sp is some variable
number of instructions into the frame and is frequently missed due to
not being on a word boundary, leading to incomplete walking of the
stack.
Fix this by incrementing the instruction pointer based on the size of
the previously decoded instruction.
Fixes: b6c7a324df37 ("MIPS: Fix get_frame_info() handling of microMIPS function size")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
Changes in v2:
- Keep locals in reverse christmas tree order
arch/mips/kernel/process.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 803e255b6fc3..df69ebd361fc 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -346,6 +346,7 @@ static int get_frame_info(struct mips_frame_info *info)
bool is_mmips = IS_ENABLED(CONFIG_CPU_MICROMIPS);
union mips_instruction insn, *ip, *ip_end;
const unsigned int max_insns = 128;
+ unsigned int last_insn_size = 0;
unsigned int i;
info->pc_offset = -1;
@@ -357,15 +358,19 @@ static int get_frame_info(struct mips_frame_info *info)
ip_end = (void *)ip + info->func_size;
- for (i = 0; i < max_insns && ip < ip_end; i++, ip++) {
+ for (i = 0; i < max_insns && ip < ip_end; i++) {
+ ip = (void *)ip + last_insn_size;
if (is_mmips && mm_insn_16bit(ip->halfword[0])) {
insn.halfword[0] = 0;
insn.halfword[1] = ip->halfword[0];
+ last_insn_size = sizeof(unsigned short);
} else if (is_mmips) {
insn.halfword[0] = ip->halfword[1];
insn.halfword[1] = ip->halfword[0];
+ last_insn_size = sizeof(unsigned int);
} else {
insn.word = ip->word;
+ last_insn_size = sizeof(unsigned int);
}
if (is_jump_ins(&insn))
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/5] MIPS: microMIPS: Fix decoding of addiusp instruction
2017-03-01 14:41 [PATCH v2 0/5] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 1/5] MIPS: Handle non word sized instructions when examining frame Matt Redfearn
@ 2017-03-01 14:41 ` Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 3/5] MIPS: Refactor handling of stack pointer in get_frame_info Matt Redfearn
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-01 14:41 UTC (permalink / raw)
To: Ralf Baechle
Cc: linux-mips, Matt Redfearn, Maciej W. Rozycki, Marcin Nowakowski,
linux-kernel, Paul Burton
Commit 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
added handling of microMIPS instructions to manipulate the stack
pointer. Unfortunately the decoding of the addiusp instruction was
incorrect, and performed a left shift by 2 bits to the raw immediate,
rather than decoding the immediate and then performing the shift, as
documented in the ISA.
This led to incomplete stack traces, due to incorrect frame sizes being
calculated. For example the instruction:
801faee0 <do_sys_poll>:
801faee0: 4e25 addiu sp,sp,-952
As decoded by objdump, would be interpreted by the existing code as
having manipulated the stack pointer by +1096.
Fix this by changing the order of decoding the immediate and applying
the left shift.
Fixes: 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
Changes in v2:
- Replace conditional with xor and subtract
arch/mips/kernel/process.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index df69ebd361fc..799273d45d21 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -386,8 +386,9 @@ static int get_frame_info(struct mips_frame_info *info)
if (ip->halfword[0] & mm_addiusp_func)
{
- tmp = (((ip->halfword[0] >> 1) & 0x1ff) << 2);
- info->frame_size = -(signed short)(tmp | ((tmp & 0x100) ? 0xfe00 : 0));
+ tmp = (ip->halfword[0] >> 1) & 0x1ff;
+ tmp = (tmp ^ 0x100) - 0x100;
+ info->frame_size = -(signed short)(tmp << 2);
} else {
tmp = (ip->halfword[0] >> 1);
info->frame_size = -(signed short)(tmp & 0xf);
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 3/5] MIPS: Refactor handling of stack pointer in get_frame_info
2017-03-01 14:41 [PATCH v2 0/5] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 1/5] MIPS: Handle non word sized instructions when examining frame Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 2/5] MIPS: microMIPS: Fix decoding of addiusp instruction Matt Redfearn
@ 2017-03-01 14:41 ` Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 4/5] MIPS: Stacktrace: Fix __usermode() of uninitialised regs Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 5/5] MIPS: microMIPS: Fix decoding of swsp16 instruction Matt Redfearn
4 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-01 14:41 UTC (permalink / raw)
To: Ralf Baechle
Cc: linux-mips, Matt Redfearn, Marcin Nowakowski, linux-kernel,
Maciej W. Rozycki, Paul Burton
Commit 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
added handling of microMIPS instructions to manipulate the stack
pointer. The code that was added violates code style rules with long
lines caused by lots of nested conditionals.
The added code interprets (inline) any known stack pointer manipulation
instruction to find the stack frame size. Handling the microMIPS cases
added quite a bit of complication to this function.
This commit refactors is_sp_move_ins to perform the interpretation of
the immediate as the instruction manipulating the stack pointer is
found. This reduces the amount of indentation required in
get_frame_info, and more closely matches the operation of
is_ra_save_ins.
Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
Changes in v2:
- Refactor is_sp_move_ins to interpret immediate inline.
arch/mips/kernel/process.c | 58 ++++++++++++++++++++++------------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 799273d45d21..1113f1f15bc1 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -311,9 +311,11 @@ static inline int is_jump_ins(union mips_instruction *ip)
#endif
}
-static inline int is_sp_move_ins(union mips_instruction *ip)
+static inline int is_sp_move_ins(union mips_instruction *ip, int *frame_size)
{
#ifdef CONFIG_CPU_MICROMIPS
+ unsigned short tmp;
+
/*
* addiusp -imm
* addius5 sp,-imm
@@ -323,20 +325,37 @@ static inline int is_sp_move_ins(union mips_instruction *ip)
* microMIPS is not more fun...
*/
if (mm_insn_16bit(ip->halfword[1])) {
- return (ip->mm16_r3_format.opcode == mm_pool16d_op &&
- ip->mm16_r3_format.simmediate && mm_addiusp_func) ||
- (ip->mm16_r5_format.opcode == mm_pool16d_op &&
- ip->mm16_r5_format.rt == 29);
+ if (ip->mm16_r3_format.opcode == mm_pool16d_op &&
+ ip->mm16_r3_format.simmediate && mm_addiusp_func) {
+ tmp = (ip->halfword[1] >> 1) & 0x1ff;
+ tmp = (tmp ^ 0x100) - 0x100;
+ *frame_size = -(signed short)(tmp << 2);
+ return 1;
+ }
+ if (ip->mm16_r5_format.opcode == mm_pool16d_op &&
+ ip->mm16_r5_format.rt == 29) {
+ tmp = (ip->halfword[1] >> 1);
+ *frame_size = -(signed short)(tmp & 0xf);
+ return 1;
+ }
+ return 0;
}
- return ip->mm_i_format.opcode == mm_addiu32_op &&
- ip->mm_i_format.rt == 29 && ip->mm_i_format.rs == 29;
+ if (ip->mm_i_format.opcode == mm_addiu32_op &&
+ ip->mm_i_format.rt == 29 && ip->mm_i_format.rs == 29) {
+ *frame_size = -ip->i_format.simmediate;
+ return 1;
+ }
#else
/* addiu/daddiu sp,sp,-imm */
if (ip->i_format.rs != 29 || ip->i_format.rt != 29)
return 0;
- if (ip->i_format.opcode == addiu_op || ip->i_format.opcode == daddiu_op)
+
+ if (ip->i_format.opcode == addiu_op ||
+ ip->i_format.opcode == daddiu_op) {
+ *frame_size = -ip->i_format.simmediate;
return 1;
+ }
#endif
return 0;
}
@@ -377,28 +396,7 @@ static int get_frame_info(struct mips_frame_info *info)
break;
if (!info->frame_size) {
- if (is_sp_move_ins(&insn))
- {
-#ifdef CONFIG_CPU_MICROMIPS
- if (mm_insn_16bit(ip->halfword[0]))
- {
- unsigned short tmp;
-
- if (ip->halfword[0] & mm_addiusp_func)
- {
- tmp = (ip->halfword[0] >> 1) & 0x1ff;
- tmp = (tmp ^ 0x100) - 0x100;
- info->frame_size = -(signed short)(tmp << 2);
- } else {
- tmp = (ip->halfword[0] >> 1);
- info->frame_size = -(signed short)(tmp & 0xf);
- }
- ip = (void *) &ip->halfword[1];
- ip--;
- } else
-#endif
- info->frame_size = - ip->i_format.simmediate;
- }
+ is_sp_move_ins(&insn, &info->frame_size);
continue;
}
if (info->pc_offset == -1 &&
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 4/5] MIPS: Stacktrace: Fix __usermode() of uninitialised regs
2017-03-01 14:41 [PATCH v2 0/5] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
` (2 preceding siblings ...)
2017-03-01 14:41 ` [PATCH v2 3/5] MIPS: Refactor handling of stack pointer in get_frame_info Matt Redfearn
@ 2017-03-01 14:41 ` Matt Redfearn
2017-03-01 14:41 ` [PATCH v2 5/5] MIPS: microMIPS: Fix decoding of swsp16 instruction Matt Redfearn
4 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-01 14:41 UTC (permalink / raw)
To: Ralf Baechle; +Cc: linux-mips, Matt Redfearn, linux-kernel
Commit 81a76d7119f6 ("MIPS: Avoid using unwind_stack() with usermode")
added a check if the passed regs are from user mode, and perform a raw
backtrace if so.
When WARN() is invoked, __dump_stack calls show_stack()
with NULL task and stack pointers. This leads show_stack to create a
pt_regs struct on the stack, and initialise it via prepare_frametrace().
When show_backtrace() examines the regs, the value of the status
register checked by user_mode() is unpredictable, depending on the
uninitialised content of the stack. This leads to show_backtrace()
sometimes showing raw backtraces instead of correctly walking the kernel
stack.
Fix this by initialising the contents of the saved status register in
prepare_frametrace().
Fixes: 81a76d7119f6 ("MIPS: Avoid using unwind_stack() with usermode")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
Changes in v2: None
arch/mips/include/asm/stacktrace.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/mips/include/asm/stacktrace.h b/arch/mips/include/asm/stacktrace.h
index 780ee2c2a2ac..4845945d02a5 100644
--- a/arch/mips/include/asm/stacktrace.h
+++ b/arch/mips/include/asm/stacktrace.h
@@ -1,6 +1,7 @@
#ifndef _ASM_STACKTRACE_H
#define _ASM_STACKTRACE_H
+#include <asm/asm.h>
#include <asm/ptrace.h>
#ifdef CONFIG_KALLSYMS
@@ -47,6 +48,8 @@ static __always_inline void prepare_frametrace(struct pt_regs *regs)
: "=m" (regs->cp0_epc),
"=m" (regs->regs[29]), "=m" (regs->regs[31])
: : "memory");
+ /* show_backtrace behaviour depends on user_mode(regs) */
+ regs->cp0_status = read_c0_status();
}
#endif /* _ASM_STACKTRACE_H */
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 5/5] MIPS: microMIPS: Fix decoding of swsp16 instruction
2017-03-01 14:41 [PATCH v2 0/5] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
` (3 preceding siblings ...)
2017-03-01 14:41 ` [PATCH v2 4/5] MIPS: Stacktrace: Fix __usermode() of uninitialised regs Matt Redfearn
@ 2017-03-01 14:41 ` Matt Redfearn
4 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-01 14:41 UTC (permalink / raw)
To: Ralf Baechle
Cc: linux-mips, Matt Redfearn, Paolo Bonzini, Marcin Nowakowski,
Huacai Chen, linux-kernel, James Hogan, Andrew Morton,
Paul Burton
When the immediate encoded in the instruction is accessed, it is sign
extended due to being a signed value being assigned to a signed integer.
The ISA specifies that this operation is an unsigned operation.
The sign extension leads us to incorrectly decode:
801e9c8e: cbf1 sw ra,68(sp)
As having an immediate of 1073741809.
Since the instruction format does not specify signed/unsigned, and this
is currently the only location to use this instuction format, change it
to an unsigned immediate.
Fixes: bb9bc4689b9c ("MIPS: Calculate microMIPS ra properly when unwinding the stack")
Suggested-by: Paul Burton <paul.burton@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
Changes in v2: None
arch/mips/include/uapi/asm/inst.h | 2 +-
arch/mips/kernel/process.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/include/uapi/asm/inst.h b/arch/mips/include/uapi/asm/inst.h
index 77429d1622b3..711d9b8465b8 100644
--- a/arch/mips/include/uapi/asm/inst.h
+++ b/arch/mips/include/uapi/asm/inst.h
@@ -964,7 +964,7 @@ struct mm16_r3_format { /* Load from global pointer format */
struct mm16_r5_format { /* Load/store from stack pointer format */
__BITFIELD_FIELD(unsigned int opcode : 6,
__BITFIELD_FIELD(unsigned int rt : 5,
- __BITFIELD_FIELD(signed int simmediate : 5,
+ __BITFIELD_FIELD(unsigned int imm : 5,
__BITFIELD_FIELD(unsigned int : 16, /* Ignored */
;))))
};
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 1113f1f15bc1..0f4c0b3769ca 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -212,7 +212,7 @@ static inline int is_ra_save_ins(union mips_instruction *ip, int *poff)
if (ip->mm16_r5_format.rt != 31)
return 0;
- *poff = ip->mm16_r5_format.simmediate;
+ *poff = ip->mm16_r5_format.imm;
*poff = (*poff << 2) / sizeof(ulong);
return 1;
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread