diff mbox

powerpc/lib/sstep.c - Fix emulation fall-through

Message ID 1455578929-29599-1-git-send-email-oohall@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Oliver O'Halloran Feb. 15, 2016, 11:28 p.m. UTC
There is a switch fallthough in instr_analyze() which can cause
an invalid instruction to be emulated as a different, valid,
instruction. The rld* (opcode 30) case extracts a sub-opcode from
bits 3:1 of the instruction word. However, the only valid values
of this field a 001 and 000. These cases are correctly handled,
but the others are not which causes execution to fall through
into case 31.

Breaking out of the switch causes the instruction to be marked as
unknown and allows the caller to deal with the invalid instruction
in a manner consistent with other invalid instructions.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/lib/sstep.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrew Donnellan Feb. 16, 2016, 12:59 a.m. UTC | #1
On 16/02/16 10:28, Oliver O'Halloran wrote:
> There is a switch fallthough in instr_analyze() which can cause
> an invalid instruction to be emulated as a different, valid,
> instruction. The rld* (opcode 30) case extracts a sub-opcode from
> bits 3:1 of the instruction word. However, the only valid values
> of this field a 001 and 000. These cases are correctly handled,
> but the others are not which causes execution to fall through
> into case 31.
>
> Breaking out of the switch causes the instruction to be marked as
> unknown and allows the caller to deal with the invalid instruction
> in a manner consistent with other invalid instructions.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

The title should probably be "powerpc/sstep: fix switch fallthrough in 
instruction emulation" to be consistent with our usual patch titling 
practice. Please respin.

Apart from that, I'm reasonably convinced this is an appropriate fix:

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
diff mbox

Patch

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index dc885b3..e25f73c 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -925,6 +925,7 @@  int __kprobes analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			}
 		}
 #endif
+	break; /* illegal instruction */
 
 	case 31:
 		switch ((instr >> 1) & 0x3ff) {