[PATCH] powerpc/kprobes: Update optprobes to use emulate_update_regs()

[PATCH] powerpc/kprobes: Update optprobes to use emulate_update_regs()

[Naveen N. Rao]

Optprobes depended on an updated regs->nip from analyse_instr() to
identify the location to branch back from the optprobes trampoline.
However, since commit 3cdfcbfd32b9d ("powerpc: Change analyse_instr so
it doesn't modify *regs"), analyse_instr() doesn't update the registers
anymore.  Due to this, we end up branching back from the optprobes
trampoline to the same branch into the trampoline resulting in a loop.

Fix this by calling out to emulate_update_regs() before using the nip.
Additionally, explicitly compare the return value from analyse_instr()
to 1, rather than just checking for !0 so as to guard against any
future changes to analyse_instr() that may result in -1 being returned
in more scenarios.

Fixes: 3cdfcbfd32b9d ("powerpc: Change analyse_instr so it doesn't modify *regs")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/optprobes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 6f8273f5e988..91e037ab20a1 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -104,8 +104,10 @@ static unsigned long can_optimize(struct kprobe *p)
 	 * and that can be emulated.
 	 */
 	if (!is_conditional_branch(*p->ainsn.insn) &&
-			analyse_instr(&op, &regs, *p->ainsn.insn))
+			analyse_instr(&op, &regs, *p->ainsn.insn) == 1) {
+		emulate_update_regs(&regs, &op);
 		nip = regs.nip;
+	}
 
 	return nip;
 }
-- 
2.14.1

Re: powerpc/kprobes: Update optprobes to use emulate_update_regs()

[Michael Ellerman]

On Fri, 2017-09-15 at 10:08:21 UTC, "Naveen N. Rao" wrote:
> Optprobes depended on an updated regs->nip from analyse_instr() to
> identify the location to branch back from the optprobes trampoline.
> However, since commit 3cdfcbfd32b9d ("powerpc: Change analyse_instr so
> it doesn't modify *regs"), analyse_instr() doesn't update the registers
> anymore.  Due to this, we end up branching back from the optprobes
> trampoline to the same branch into the trampoline resulting in a loop.
> 
> Fix this by calling out to emulate_update_regs() before using the nip.
> Additionally, explicitly compare the return value from analyse_instr()
> to 1, rather than just checking for !0 so as to guard against any
> future changes to analyse_instr() that may result in -1 being returned
> in more scenarios.
> 
> Fixes: 3cdfcbfd32b9d ("powerpc: Change analyse_instr so it doesn't modify *regs")
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/8afafa6fba7809c0785018b77c95b1

cheers