| Message ID | 1468937260-23988-3-git-send-email-will.deacon@arm.com |
|---|---|
| State | Accepted |
| Commit | 44bd887ce10eb8061f6a137f8a73f823957edd82 |
| Headers | show |
On Tue, Jul 19, 2016 at 03:07:39PM +0100, Will Deacon wrote: > Stepping with PSTATE.D=1 is bad news. The step won't generate a debug > exception and we'll likely walk off into random data structures. This > should never happen, but when it does, it's a PITA to debug. Add a > WARN_ON to shout if we realise this is about to take place. > > Signed-off-by: Will Deacon <will.deacon@arm.com> Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > arch/arm64/kernel/probes/kprobes.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c > index 9c70e8812ea9..c89811d1e294 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -254,6 +254,8 @@ static void __kprobes setup_singlestep(struct kprobe *p, > > if (kcb->kprobe_status == KPROBE_REENTER) > spsr_set_debug_flag(regs, 0); > + else > + WARN_ON(regs->pstate & PSR_D_BIT); > > /* IRQs and single stepping do not mix well. */ > kprobes_save_local_irqflag(kcb, regs); > -- > 2.1.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Aug 03, 2016 at 05:52:55PM +0530, Pratyush Anand wrote: > Hi Will, > > Its already in torvalds/linux.git: master now. I have some related > queries, so thought to discuss it here. > > On Tue, Jul 19, 2016 at 7:37 PM, Will Deacon <will.deacon@arm.com> wrote: > > Stepping with PSTATE.D=1 is bad news. The step won't generate a debug > > exception and we'll likely walk off into random data structures. This > > should never happen, but when it does, it's a PITA to debug. Add a > > But it happens in many know scenarios, like: > > 1) We are executing a WARN_ON(), which will call `BRK BUG_BRK_IMM`. > It prints warning messages through breakpoint handler. Now, suppose we > have a kprobe instrumented at a print function branch, say > print_worker_info(), we will land into > kprobe_handler()->setup_singlestep() with D-bit set. In this case if > we do not clear it, then we receive undefined exception before we > could get single step exception. > > 2) Similarly, if we instrument kprobe at uprobe_breakpoint_handler() > (code not yet in upstream), we land into similar situation which > leads to infinite "Unexpected kernel single-step exception at EL1". > > So, why can't we clear PSR_D_BIT in setup_singlestep unconditionally? > I found that both of the above issue is resolved by doing that. I think that will work, but the advantage of the WARN_ON is that it can highlight cases where kprobes have been placed on the debug exception path, which is generally a Bad Idea as it can result in infinite recursion loops. I know that __kprobes is supposed to deal with this, but in reality that's all a best guess and looks to be incomplete. If we can do a better job of annotating the debug exception path, I'd be up for unconditional clearing of PSR_D_BIT in the target when returning. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 9c70e8812ea9..c89811d1e294 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -254,6 +254,8 @@ static void __kprobes setup_singlestep(struct kprobe *p, if (kcb->kprobe_status == KPROBE_REENTER) spsr_set_debug_flag(regs, 0); + else + WARN_ON(regs->pstate & PSR_D_BIT); /* IRQs and single stepping do not mix well. */ kprobes_save_local_irqflag(kcb, regs);
Stepping with PSTATE.D=1 is bad news. The step won't generate a debug exception and we'll likely walk off into random data structures. This should never happen, but when it does, it's a PITA to debug. Add a WARN_ON to shout if we realise this is about to take place. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm64/kernel/probes/kprobes.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel