[PATCH v10 6/9] arm64: kprobes instruction simulation support

On Thu, 3 Mar 2016 00:02:43 -0500
David Long [off-list ref] wrote:

On 03/01/2016 01:04 PM, Marc Zyngier wrote:

quoted

On 01/03/16 02:57, David Long wrote:

quoted

From: Sandeepa Prabhu <redacted>

Kprobes needs simulation of instructions that cannot be stepped
from different memory location, e.g.: those instructions
that uses PC-relative addressing. In simulation, the behaviour
of the instruction is implemented using a copy of pt_regs.

Following instruction catagories are simulated:
  - All branching instructions(conditional, register, and immediate)
  - Literal access instructions(load-literal, adr/adrp)

Conditional execution is limited to branching instructions in
ARM v8. If conditions at PSTATE do not match the condition fields
of opcode, the instruction is effectively NOP. Kprobes considers
this case as 'miss'.

This code also replaces the use of arch/arm/opcodes.c for
arm_check_condition().

Outdated comment?

Yeah.  I'll remove it.

quoted

quoted

Thanks to Will Cohen for assorted suggested changes.

Signed-off-by: Sandeepa Prabhu <redacted>
Signed-off-by: William Cohen <redacted>
Signed-off-by: David A. Long <redacted>
---
  arch/arm64/include/asm/insn.h            |   1 +
  arch/arm64/include/asm/probes.h          |   5 +-
  arch/arm64/kernel/Makefile               |   3 +-
  arch/arm64/kernel/insn.c                 |   1 +
  arch/arm64/kernel/kprobes-arm64.c        |  29 +++++
  arch/arm64/kernel/kprobes.c              |  32 +++++-
  arch/arm64/kernel/probes-simulate-insn.c | 187 +++++++++++++++++++++++++++++++
  arch/arm64/kernel/probes-simulate-insn.h |  28 +++++
  8 files changed, 280 insertions(+), 6 deletions(-)
  create mode 100644 arch/arm64/kernel/probes-simulate-insn.c
  create mode 100644 arch/arm64/kernel/probes-simulate-insn.h

[...]

quoted

quoted

+/*
+ * instruction simulation functions
+ */
+void __kprobes
+simulate_adr_adrp(u32 opcode, long addr, struct pt_regs *regs)
+{
+	long imm, xn, val;
+
+	xn = opcode & 0x1f;
+	imm = ((opcode >> 3) & 0x1ffffc) | ((opcode >> 29) & 0x3);
+	imm = sign_extend(imm, 20);
+	if (opcode & 0x80000000)
+		val = (imm<<12) + (addr & 0xfffffffffffff000);
+	else
+		val = imm + addr;
+
+	regs->regs[xn] = val;

What happens when you have something like "adr xzr, blah"? I haven't
found out where you are writing that back yet, but that could be really
fun for SP...

It hadn't occurred to me that xzr could be an output register. Sigh. 
That could mean a bit of repeated code to handle this special case.  I 
wonder what the implications would be of adding xzr to the pt_regs 
structure to avoid that.

xzr is not a register. It is an encoding that tells the CPU to discard
the result of an operation. As such, there is no need to store it.

An easy fix for this would be to have an accessor that actually checks
for the register number, and only allows the range 0-30. We've used
similar things in KVM for the same reasons (vcpu_get_reg/vcpu_set_reg).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.