All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>
Cc: mhiramat@kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Naresh Kamboju <naresh.kamboju@linaro.org>,
	Dan Rue <dan.rue@linaro.org>, Matt Hart <matthew.hart@linaro.org>,
	Anders Roxell <anders.roxell@linaro.org>,
	Daniel Diaz <daniel.diaz@linaro.org>
Subject: [PATCH 1/3] arm64: kprobes: Recover pstate.D in single-step exception handler
Date: Thu, 18 Jul 2019 14:43:37 +0900	[thread overview]
Message-ID: <156342861775.8565.9122725195458920037.stgit@devnote2> (raw)
In-Reply-To: <156342860634.8565.14804606041960884732.stgit@devnote2>

On arm64, if a nested kprobes hit, it can crash the kernel with below
error message.

[  152.118921] Unexpected kernel single-step exception at EL1

This is because commit 7419333fa15e ("arm64: kprobe: Always clear
pstate.D in breakpoint exception handler") clears pstate.D always in
the nested kprobes. That is correct *unless* any nested kprobes
(single-stepping) runs inside other kprobes (including kprobes in
 user handler).

When the 1st kprobe hits, do_debug_exception() will be called. At this
point, debug exception (= pstate.D) must be masked (=1). When the 2nd
 (nested) kprobe is hit before single-step of the first kprobe, it
modifies debug exception clear (pstate.D = 0).
Then, when the 1st kprobe setting up single-step, it saves current
DAIF, mask DAIF, enable single-step, and restore DAIF.
However, since "D" flag in DAIF is cleared by the 2nd kprobe, the
single-step exception happens soon after restoring DAIF.

To solve this issue, this refers saved pstate register to check the
previous pstate.D and recover it if needed.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: commit 7419333fa15e ("arm64: kprobe: Always clear pstate.D in breakpoint exception handler")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/kernel/probes/kprobes.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index bd5dfffca272..6e1dc0bb4c82 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -201,12 +201,14 @@ spsr_set_debug_flag(struct pt_regs *regs, int mask)
  * interrupt occurrence in the period of exception return and  start of
  * out-of-line single-step, that result in wrongly single stepping
  * into the interrupt handler.
+ * This also controls debug flag, so that we can refer the saved pstate.
  */
 static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
 						struct pt_regs *regs)
 {
 	kcb->saved_irqflag = regs->pstate;
 	regs->pstate |= PSR_I_BIT;
+	spsr_set_debug_flag(regs, 0);
 }
 
 static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
@@ -216,6 +218,10 @@ static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
 		regs->pstate |= PSR_I_BIT;
 	else
 		regs->pstate &= ~PSR_I_BIT;
+
+	/* Recover pstate.D mask if needed */
+	if (kcb->saved_irqflag & PSR_D_BIT)
+		spsr_set_debug_flag(regs, 1);
 }
 
 static void __kprobes
@@ -245,15 +251,12 @@ static void __kprobes setup_singlestep(struct kprobe *p,
 		kcb->kprobe_status = KPROBE_HIT_SS;
 	}
 
-
 	if (p->ainsn.api.insn) {
 		/* prepare for single stepping */
 		slot = (unsigned long)p->ainsn.api.insn;
 
 		set_ss_context(kcb, slot);	/* mark pending ss */
 
-		spsr_set_debug_flag(regs, 0);
-
 		/* IRQs and single stepping do not mix well. */
 		kprobes_save_local_irqflag(kcb, regs);
 		kernel_enable_single_step(regs);


WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>
Cc: Dan Rue <dan.rue@linaro.org>,
	Daniel Diaz <daniel.diaz@linaro.org>,
	Anders Roxell <anders.roxell@linaro.org>,
	Naresh Kamboju <naresh.kamboju@linaro.org>,
	linux-kernel@vger.kernel.org, Matt Hart <matthew.hart@linaro.org>,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org
Subject: [PATCH 1/3] arm64: kprobes: Recover pstate.D in single-step exception handler
Date: Thu, 18 Jul 2019 14:43:37 +0900	[thread overview]
Message-ID: <156342861775.8565.9122725195458920037.stgit@devnote2> (raw)
In-Reply-To: <156342860634.8565.14804606041960884732.stgit@devnote2>

On arm64, if a nested kprobes hit, it can crash the kernel with below
error message.

[  152.118921] Unexpected kernel single-step exception at EL1

This is because commit 7419333fa15e ("arm64: kprobe: Always clear
pstate.D in breakpoint exception handler") clears pstate.D always in
the nested kprobes. That is correct *unless* any nested kprobes
(single-stepping) runs inside other kprobes (including kprobes in
 user handler).

When the 1st kprobe hits, do_debug_exception() will be called. At this
point, debug exception (= pstate.D) must be masked (=1). When the 2nd
 (nested) kprobe is hit before single-step of the first kprobe, it
modifies debug exception clear (pstate.D = 0).
Then, when the 1st kprobe setting up single-step, it saves current
DAIF, mask DAIF, enable single-step, and restore DAIF.
However, since "D" flag in DAIF is cleared by the 2nd kprobe, the
single-step exception happens soon after restoring DAIF.

To solve this issue, this refers saved pstate register to check the
previous pstate.D and recover it if needed.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: commit 7419333fa15e ("arm64: kprobe: Always clear pstate.D in breakpoint exception handler")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/kernel/probes/kprobes.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index bd5dfffca272..6e1dc0bb4c82 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -201,12 +201,14 @@ spsr_set_debug_flag(struct pt_regs *regs, int mask)
  * interrupt occurrence in the period of exception return and  start of
  * out-of-line single-step, that result in wrongly single stepping
  * into the interrupt handler.
+ * This also controls debug flag, so that we can refer the saved pstate.
  */
 static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
 						struct pt_regs *regs)
 {
 	kcb->saved_irqflag = regs->pstate;
 	regs->pstate |= PSR_I_BIT;
+	spsr_set_debug_flag(regs, 0);
 }
 
 static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
@@ -216,6 +218,10 @@ static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
 		regs->pstate |= PSR_I_BIT;
 	else
 		regs->pstate &= ~PSR_I_BIT;
+
+	/* Recover pstate.D mask if needed */
+	if (kcb->saved_irqflag & PSR_D_BIT)
+		spsr_set_debug_flag(regs, 1);
 }
 
 static void __kprobes
@@ -245,15 +251,12 @@ static void __kprobes setup_singlestep(struct kprobe *p,
 		kcb->kprobe_status = KPROBE_HIT_SS;
 	}
 
-
 	if (p->ainsn.api.insn) {
 		/* prepare for single stepping */
 		slot = (unsigned long)p->ainsn.api.insn;
 
 		set_ss_context(kcb, slot);	/* mark pending ss */
 
-		spsr_set_debug_flag(regs, 0);
-
 		/* IRQs and single stepping do not mix well. */
 		kprobes_save_local_irqflag(kcb, regs);
 		kernel_enable_single_step(regs);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-07-18  5:43 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-18  5:43 [PATCH 0/3] arm64: kprobes: Fix some bugs in arm64 kprobes Masami Hiramatsu
2019-07-18  5:43 ` Masami Hiramatsu
2019-07-18  5:43 ` Masami Hiramatsu [this message]
2019-07-18  5:43   ` [PATCH 1/3] arm64: kprobes: Recover pstate.D in single-step exception handler Masami Hiramatsu
2019-07-19 10:07   ` James Morse
2019-07-19 10:07     ` James Morse
2019-07-20  6:22     ` Masami Hiramatsu
2019-07-20  6:22       ` Masami Hiramatsu
2019-07-18  5:43 ` [PATCH 2/3] arm64: unwind: Prohibit probing on return_address() Masami Hiramatsu
2019-07-18  5:43   ` Masami Hiramatsu
2019-07-18  5:43 ` [PATCH 3/3] arm64: debug: Remove rcu_read_lock from debug exception Masami Hiramatsu
2019-07-18  5:43   ` Masami Hiramatsu
2019-07-18  6:22   ` Paul E. McKenney
2019-07-18  6:22     ` Paul E. McKenney
2019-07-18  9:20     ` Mark Rutland
2019-07-18  9:20       ` Mark Rutland
2019-07-18 14:31       ` Masami Hiramatsu
2019-07-18 14:31         ` Masami Hiramatsu
2019-07-19  8:42         ` James Morse
2019-07-19  8:42           ` James Morse
2019-07-20  7:32           ` Masami Hiramatsu
2019-07-20  7:32             ` Masami Hiramatsu
2019-07-21  1:50             ` Masami Hiramatsu
2019-07-21  1:50               ` Masami Hiramatsu
2019-07-19  9:59         ` Mark Rutland
2019-07-19  9:59           ` Mark Rutland
2019-07-20  7:54           ` Masami Hiramatsu
2019-07-20  7:54             ` Masami Hiramatsu
2019-07-24 10:45             ` Mark Rutland
2019-07-24 10:45               ` Mark Rutland

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=156342861775.8565.9122725195458920037.stgit@devnote2 \
    --to=mhiramat@kernel.org \
    --cc=anders.roxell@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=dan.rue@linaro.org \
    --cc=daniel.diaz@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew.hart@linaro.org \
    --cc=naresh.kamboju@linaro.org \
    --cc=will.deacon@arm.com \
    /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.