All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Jordan Niethe <jniethe5@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Subject: [PATCH 3/5] powerpc/kprobes: Check instruction validity during kprobe registration
Date: Wed, 19 May 2021 16:17:19 +0530	[thread overview]
Message-ID: <f033696e8ac89a7cba2c336680dfb67911c643bd.1621416666.git.naveen.n.rao@linux.vnet.ibm.com> (raw)
In-Reply-To: <cover.1621416666.git.naveen.n.rao@linux.vnet.ibm.com>

In trap-based (classic) kprobes, we try to emulate the probed
instruction so as to avoid having to single step it. We use a flag to
determine if the probed instruction was successfully emulated, so that we
can speed up subsequent probe hits.

However, emulate_step() doesn't differentiate between unknown
instructions and an emulation attempt that failed. As such, the current
heuristic is not of much use. Instead, use analyse_instr() during kprobe
registration to determine if the probed instruction can be decoded by
our instruction emulation infrastructure. For unknown instructions, we
can then directly single-step while for other instructions, we can
attempt to emulate and fall back to single stepping if that fails.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 62 +++++++++--------------------------
 1 file changed, 16 insertions(+), 46 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index b7b20875d34d91..bbef9e918ecb39 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -107,6 +107,8 @@ int arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
 	struct kprobe *prev;
+	struct pt_regs regs;
+	struct instruction_op op;
 	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
 
 	if ((unsigned long)p->addr & 0x03) {
@@ -140,9 +142,18 @@ int arch_prepare_kprobe(struct kprobe *p)
 	if (!ret) {
 		patch_instruction((struct ppc_inst *)p->ainsn.insn, insn);
 		p->opcode = ppc_inst_val(insn);
+
+		/* Check if this is an instruction we recognise */
+		p->ainsn.boostable = 0;
+		memset(&regs, 0, sizeof(struct pt_regs));
+		regs.nip = (unsigned long)p->addr;
+		regs.msr = MSR_KERNEL;
+		ret = analyse_instr(&op, &regs, insn);
+		if (ret == 1 || (ret == 0 && GETTYPE(op.type) != UNKNOWN))
+			p->ainsn.boostable = 1;
+		ret = 0;
 	}
 
-	p->ainsn.boostable = 0;
 	return ret;
 }
 NOKPROBE_SYMBOL(arch_prepare_kprobe);
@@ -225,47 +236,6 @@ void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(arch_prepare_kretprobe);
 
-static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
-{
-	int ret;
-	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
-
-	/* regs->nip is also adjusted if emulate_step returns 1 */
-	ret = emulate_step(regs, insn);
-	if (ret > 0) {
-		/*
-		 * Once this instruction has been boosted
-		 * successfully, set the boostable flag
-		 */
-		if (unlikely(p->ainsn.boostable == 0))
-			p->ainsn.boostable = 1;
-	} else if (ret < 0) {
-		/*
-		 * We don't allow kprobes on mtmsr(d)/rfi(d), etc.
-		 * So, we should never get here... but, its still
-		 * good to catch them, just in case...
-		 */
-		printk("Can't step on instruction %s\n", ppc_inst_as_str(insn));
-		BUG();
-	} else {
-		/*
-		 * If we haven't previously emulated this instruction, then it
-		 * can't be boosted. Note it down so we don't try to do so again.
-		 *
-		 * If, however, we had emulated this instruction in the past,
-		 * then this is just an error with the current run (for
-		 * instance, exceptions due to a load/store). We return 0 so
-		 * that this is now single-stepped, but continue to try
-		 * emulating it in subsequent probe hits.
-		 */
-		if (unlikely(p->ainsn.boostable != 1))
-			p->ainsn.boostable = -1;
-	}
-
-	return ret;
-}
-NOKPROBE_SYMBOL(try_to_emulate);
-
 int kprobe_handler(struct pt_regs *regs)
 {
 	struct kprobe *p;
@@ -334,8 +304,8 @@ int kprobe_handler(struct pt_regs *regs)
 		set_current_kprobe(p, regs, kcb);
 		kprobes_inc_nmissed_count(p);
 		kcb->kprobe_status = KPROBE_REENTER;
-		if (p->ainsn.boostable >= 0) {
-			ret = try_to_emulate(p, regs);
+		if (p->ainsn.boostable) {
+			ret = emulate_step(regs, ppc_inst_read((struct ppc_inst *)p->ainsn.insn));
 
 			if (ret > 0) {
 				restore_previous_kprobe(kcb);
@@ -356,8 +326,8 @@ int kprobe_handler(struct pt_regs *regs)
 		return 1;
 	}
 
-	if (p->ainsn.boostable >= 0) {
-		ret = try_to_emulate(p, regs);
+	if (p->ainsn.boostable) {
+		ret = emulate_step(regs, ppc_inst_read((struct ppc_inst *)p->ainsn.insn));
 
 		if (ret > 0) {
 			if (p->post_handler)
-- 
2.30.2


  parent reply	other threads:[~2021-05-19 10:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-19 10:47 [PATCH 0/5] powerpc/kprobes: fixes and cleanups Naveen N. Rao
2021-05-19 10:47 ` [PATCH 1/5] powerpc/kprobes: Fix validation of prefixed instructions across page boundary Naveen N. Rao
2021-05-19 10:47 ` [PATCH 2/5] powerpc/kprobes: Roll IS_RFI() macro into IS_RFID() Naveen N. Rao
2021-05-19 10:47 ` Naveen N. Rao [this message]
2021-05-19 10:47 ` [PATCH 4/5] powerpc/kprobes: Refactor arch_prepare_kprobe() Naveen N. Rao
2021-05-19 10:47 ` [PATCH 5/5] powerpc/kprobes: Warn if instruction patching failed Naveen N. Rao
2021-06-06 11:34 ` [PATCH 0/5] powerpc/kprobes: fixes and cleanups Michael Ellerman
2021-06-26 10:37 ` (subset) " Michael Ellerman
2021-06-26 10:46 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f033696e8ac89a7cba2c336680dfb67911c643bd.1621416666.git.naveen.n.rao@linux.vnet.ibm.com \
    --to=naveen.n.rao@linux.vnet.ibm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=jniethe5@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.