* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-08-22 6:46 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-22 6:46 UTC (permalink / raw) To: linux-arm-kernel Whenever we are hitting a kprobe from a none-kprobe debug exception handler, we hit an infinite occurrences of "Unexpected kernel single-step exception at EL1" PSTATE.D is debug exception mask bit. It is set whenever we enter into an exception mode. When it is set then Watchpoint, Breakpoint, and Software Step exceptions are masked. However, software Breakpoint Instruction exceptions can never be masked. Therefore, if we ever execute a BRK instruction, irrespective of D-bit setting, we will be receiving a corresponding breakpoint exception. For example: - We are executing kprobe pre/post handler, and kprobe has been inserted in one of the instruction of a function called by handler. So, it executes BRK instruction and we land into the case of KPROBE_REENTER. (This case is already handled by current code) - We are executing uprobe handler or any other BRK handler such as in WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we enter into kprobe breakpoint handler,from another BRK handler.(This case is not being handled currently) In all such cases kprobe breakpoint exception will be raised when we were already in debug exception mode. SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the exception was taken.So, in above example cases we would find it set in kprobe breakpoint handler. Single step exception will always be followed by a kprobe breakpoint exception.However, it will only be raised gracefully if we clear D bit while returning from breakpoint exception. If D bit is set then, it results into undefined exception and when it's handler enables dbg then single step exception is generated, however it will never be handled(because address does not match and therefore treated as unexpected). This patch clears D-flag unconditionally in setup_singlestep, so that we can always get single step exception correctly after returning from breakpoint exception. Additionally, it also removes D-flag set statement for KPROBE_REENTER return path, because debug exception for KPROBE_REENTER will always take place in a debug exception state. So, D-flag will already be set in this case. Signed-off-by: Pratyush Anand <panand@redhat.com> --- arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index e51f4ad97525..6e13361b9c01 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct kprobe *p) } /* - * The D-flag (Debug mask) is set (masked) upon debug exception entry. - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive - * probe i.e. when probe hit from kprobe handler context upon - * executing the pre/post handlers. In this case we return with - * D-flag clear so that single-stepping can be carried-out. - * - * Leave D-flag set in all other cases. + * When PSTATE.D is set (masked), then software step exceptions can not be + * generated. + * SPSR's D bit shows the value of PSTATE.D immediately before the + * exception was taken. PSTATE.D is set while entering into any exception + * mode, however software clears it for any normal (none-debug-exception) + * mode in the exception entry. Therefore, when we are entering into kprobe + * breakpoint handler from any normal mode then SPSR.D bit is already + * cleared, however it is set when we are entering from any debug exception + * mode. + * Since we always need to generate single step exception after a kprobe + * breakpoint exception therefore we need to clear it unconditionally, when + * we become sure that the current breakpoint exception is for kprobe. */ static void __kprobes spsr_set_debug_flag(struct pt_regs *regs, int mask) @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe *p, set_ss_context(kcb, slot); /* mark pending ss */ - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 0); - else - WARN_ON(regs->pstate & PSR_D_BIT); + spsr_set_debug_flag(regs, 0); /* IRQs and single stepping do not mix well. */ kprobes_save_local_irqflag(kcb, regs); @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) BUG(); kernel_disable_single_step(); - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 1); if (kcb->kprobe_status == KPROBE_REENTER) restore_previous_kprobe(kcb); @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) kprobes_restore_local_irqflag(kcb, regs); kernel_disable_single_step(); - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 1); - post_kprobe_handler(kcb, regs); } -- 2.5.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-08-22 6:46 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-22 6:46 UTC (permalink / raw) To: will.deacon Cc: catalin.marinas, dave.long, linux-arm-kernel, wcohen, mhiramat, Pratyush Anand, open list, Marc Zyngier, Sandeepa Prabhu Whenever we are hitting a kprobe from a none-kprobe debug exception handler, we hit an infinite occurrences of "Unexpected kernel single-step exception at EL1" PSTATE.D is debug exception mask bit. It is set whenever we enter into an exception mode. When it is set then Watchpoint, Breakpoint, and Software Step exceptions are masked. However, software Breakpoint Instruction exceptions can never be masked. Therefore, if we ever execute a BRK instruction, irrespective of D-bit setting, we will be receiving a corresponding breakpoint exception. For example: - We are executing kprobe pre/post handler, and kprobe has been inserted in one of the instruction of a function called by handler. So, it executes BRK instruction and we land into the case of KPROBE_REENTER. (This case is already handled by current code) - We are executing uprobe handler or any other BRK handler such as in WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we enter into kprobe breakpoint handler,from another BRK handler.(This case is not being handled currently) In all such cases kprobe breakpoint exception will be raised when we were already in debug exception mode. SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the exception was taken.So, in above example cases we would find it set in kprobe breakpoint handler. Single step exception will always be followed by a kprobe breakpoint exception.However, it will only be raised gracefully if we clear D bit while returning from breakpoint exception. If D bit is set then, it results into undefined exception and when it's handler enables dbg then single step exception is generated, however it will never be handled(because address does not match and therefore treated as unexpected). This patch clears D-flag unconditionally in setup_singlestep, so that we can always get single step exception correctly after returning from breakpoint exception. Additionally, it also removes D-flag set statement for KPROBE_REENTER return path, because debug exception for KPROBE_REENTER will always take place in a debug exception state. So, D-flag will already be set in this case. Signed-off-by: Pratyush Anand <panand@redhat.com> --- arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index e51f4ad97525..6e13361b9c01 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct kprobe *p) } /* - * The D-flag (Debug mask) is set (masked) upon debug exception entry. - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive - * probe i.e. when probe hit from kprobe handler context upon - * executing the pre/post handlers. In this case we return with - * D-flag clear so that single-stepping can be carried-out. - * - * Leave D-flag set in all other cases. + * When PSTATE.D is set (masked), then software step exceptions can not be + * generated. + * SPSR's D bit shows the value of PSTATE.D immediately before the + * exception was taken. PSTATE.D is set while entering into any exception + * mode, however software clears it for any normal (none-debug-exception) + * mode in the exception entry. Therefore, when we are entering into kprobe + * breakpoint handler from any normal mode then SPSR.D bit is already + * cleared, however it is set when we are entering from any debug exception + * mode. + * Since we always need to generate single step exception after a kprobe + * breakpoint exception therefore we need to clear it unconditionally, when + * we become sure that the current breakpoint exception is for kprobe. */ static void __kprobes spsr_set_debug_flag(struct pt_regs *regs, int mask) @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe *p, set_ss_context(kcb, slot); /* mark pending ss */ - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 0); - else - WARN_ON(regs->pstate & PSR_D_BIT); + spsr_set_debug_flag(regs, 0); /* IRQs and single stepping do not mix well. */ kprobes_save_local_irqflag(kcb, regs); @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) BUG(); kernel_disable_single_step(); - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 1); if (kcb->kprobe_status == KPROBE_REENTER) restore_previous_kprobe(kcb); @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) kprobes_restore_local_irqflag(kcb, regs); kernel_disable_single_step(); - if (kcb->kprobe_status == KPROBE_REENTER) - spsr_set_debug_flag(regs, 1); - post_kprobe_handler(kcb, regs); } -- 2.5.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler 2016-08-22 6:46 ` Pratyush Anand @ 2016-08-22 15:05 ` Masami Hiramatsu -1 siblings, 0 replies; 14+ messages in thread From: Masami Hiramatsu @ 2016-08-22 15:05 UTC (permalink / raw) To: linux-arm-kernel On Mon, 22 Aug 2016 12:16:00 +0530 Pratyush Anand <panand@redhat.com> wrote: > Whenever we are hitting a kprobe from a none-kprobe debug exception > handler, we hit an infinite occurrences of "Unexpected kernel single-step > exception at EL1" > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > exception mode. When it is set then Watchpoint, Breakpoint, and Software > Step exceptions are masked. However, software Breakpoint Instruction > exceptions can never be masked. Therefore, if we ever execute a BRK > instruction, irrespective of D-bit setting, we will be receiving a > corresponding breakpoint exception. > For example: > - We are executing kprobe pre/post handler, and kprobe has been inserted in > one of the instruction of a function called by handler. So, it executes > BRK instruction and we land into the case of KPROBE_REENTER. (This case is > already handled by current code) > - We are executing uprobe handler or any other BRK handler such as in > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > enter into kprobe breakpoint handler,from another BRK handler.(This case > is not being handled currently) > In all such cases kprobe breakpoint exception will be raised when we were > already in debug exception mode. > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > exception was taken.So, in above example cases we would find it set in > kprobe breakpoint handler. > Single step exception will always be followed by a kprobe breakpoint > exception.However, it will only be raised gracefully if we clear D bit > while returning from breakpoint exception. If D bit is set then, it results > into undefined exception and when it's handler enables dbg then single step > exception is generated, however it will never be handled(because address > does not match and therefore treated as unexpected). > This patch clears D-flag unconditionally in setup_singlestep, so that we > can always get single step exception correctly after returning from > breakpoint exception. > Additionally, it also removes D-flag set statement for KPROBE_REENTER > return path, because debug exception for KPROBE_REENTER will always take > place in a debug exception state. So, D-flag will already be set in this > case. OK, It explains this issue clearly. > > Signed-off-by: Pratyush Anand <panand@redhat.com> Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks! > --- > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > 1 file changed, 13 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c > index e51f4ad97525..6e13361b9c01 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct kprobe *p) > } > > /* > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > - * probe i.e. when probe hit from kprobe handler context upon > - * executing the pre/post handlers. In this case we return with > - * D-flag clear so that single-stepping can be carried-out. > - * > - * Leave D-flag set in all other cases. > + * When PSTATE.D is set (masked), then software step exceptions can not be > + * generated. > + * SPSR's D bit shows the value of PSTATE.D immediately before the > + * exception was taken. PSTATE.D is set while entering into any exception > + * mode, however software clears it for any normal (none-debug-exception) > + * mode in the exception entry. Therefore, when we are entering into kprobe > + * breakpoint handler from any normal mode then SPSR.D bit is already > + * cleared, however it is set when we are entering from any debug exception > + * mode. > + * Since we always need to generate single step exception after a kprobe > + * breakpoint exception therefore we need to clear it unconditionally, when > + * we become sure that the current breakpoint exception is for kprobe. > */ > static void __kprobes > spsr_set_debug_flag(struct pt_regs *regs, int mask) > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe *p, > > set_ss_context(kcb, slot); /* mark pending ss */ > > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 0); > - else > - WARN_ON(regs->pstate & PSR_D_BIT); > + spsr_set_debug_flag(regs, 0); > > /* IRQs and single stepping do not mix well. */ > kprobes_save_local_irqflag(kcb, regs); > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) > BUG(); > > kernel_disable_single_step(); > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 1); > > if (kcb->kprobe_status == KPROBE_REENTER) > restore_previous_kprobe(kcb); > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) > kprobes_restore_local_irqflag(kcb, regs); > kernel_disable_single_step(); > > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 1); > - > post_kprobe_handler(kcb, regs); > } > > -- > 2.5.5 > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-08-22 15:05 ` Masami Hiramatsu 0 siblings, 0 replies; 14+ messages in thread From: Masami Hiramatsu @ 2016-08-22 15:05 UTC (permalink / raw) To: Pratyush Anand Cc: will.deacon, catalin.marinas, dave.long, linux-arm-kernel, wcohen, mhiramat, open list, Marc Zyngier, Sandeepa Prabhu On Mon, 22 Aug 2016 12:16:00 +0530 Pratyush Anand <panand@redhat.com> wrote: > Whenever we are hitting a kprobe from a none-kprobe debug exception > handler, we hit an infinite occurrences of "Unexpected kernel single-step > exception at EL1" > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > exception mode. When it is set then Watchpoint, Breakpoint, and Software > Step exceptions are masked. However, software Breakpoint Instruction > exceptions can never be masked. Therefore, if we ever execute a BRK > instruction, irrespective of D-bit setting, we will be receiving a > corresponding breakpoint exception. > For example: > - We are executing kprobe pre/post handler, and kprobe has been inserted in > one of the instruction of a function called by handler. So, it executes > BRK instruction and we land into the case of KPROBE_REENTER. (This case is > already handled by current code) > - We are executing uprobe handler or any other BRK handler such as in > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > enter into kprobe breakpoint handler,from another BRK handler.(This case > is not being handled currently) > In all such cases kprobe breakpoint exception will be raised when we were > already in debug exception mode. > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > exception was taken.So, in above example cases we would find it set in > kprobe breakpoint handler. > Single step exception will always be followed by a kprobe breakpoint > exception.However, it will only be raised gracefully if we clear D bit > while returning from breakpoint exception. If D bit is set then, it results > into undefined exception and when it's handler enables dbg then single step > exception is generated, however it will never be handled(because address > does not match and therefore treated as unexpected). > This patch clears D-flag unconditionally in setup_singlestep, so that we > can always get single step exception correctly after returning from > breakpoint exception. > Additionally, it also removes D-flag set statement for KPROBE_REENTER > return path, because debug exception for KPROBE_REENTER will always take > place in a debug exception state. So, D-flag will already be set in this > case. OK, It explains this issue clearly. > > Signed-off-by: Pratyush Anand <panand@redhat.com> Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks! > --- > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > 1 file changed, 13 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c > index e51f4ad97525..6e13361b9c01 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct kprobe *p) > } > > /* > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > - * probe i.e. when probe hit from kprobe handler context upon > - * executing the pre/post handlers. In this case we return with > - * D-flag clear so that single-stepping can be carried-out. > - * > - * Leave D-flag set in all other cases. > + * When PSTATE.D is set (masked), then software step exceptions can not be > + * generated. > + * SPSR's D bit shows the value of PSTATE.D immediately before the > + * exception was taken. PSTATE.D is set while entering into any exception > + * mode, however software clears it for any normal (none-debug-exception) > + * mode in the exception entry. Therefore, when we are entering into kprobe > + * breakpoint handler from any normal mode then SPSR.D bit is already > + * cleared, however it is set when we are entering from any debug exception > + * mode. > + * Since we always need to generate single step exception after a kprobe > + * breakpoint exception therefore we need to clear it unconditionally, when > + * we become sure that the current breakpoint exception is for kprobe. > */ > static void __kprobes > spsr_set_debug_flag(struct pt_regs *regs, int mask) > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe *p, > > set_ss_context(kcb, slot); /* mark pending ss */ > > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 0); > - else > - WARN_ON(regs->pstate & PSR_D_BIT); > + spsr_set_debug_flag(regs, 0); > > /* IRQs and single stepping do not mix well. */ > kprobes_save_local_irqflag(kcb, regs); > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) > BUG(); > > kernel_disable_single_step(); > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 1); > > if (kcb->kprobe_status == KPROBE_REENTER) > restore_previous_kprobe(kcb); > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr) > kprobes_restore_local_irqflag(kcb, regs); > kernel_disable_single_step(); > > - if (kcb->kprobe_status == KPROBE_REENTER) > - spsr_set_debug_flag(regs, 1); > - > post_kprobe_handler(kcb, regs); > } > > -- > 2.5.5 > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <CAAOGbpwcx3G8Jdf1AfNRLrkZJQLaCcLWQPRppjvpy3=hSrffjg@mail.gmail.com>]
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler [not found] ` <CAAOGbpwcx3G8Jdf1AfNRLrkZJQLaCcLWQPRppjvpy3=hSrffjg@mail.gmail.com> @ 2016-08-24 10:06 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-24 10:06 UTC (permalink / raw) To: linux-arm-kernel Hi Sandeepa, Thanks for the review. On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: > Thanks for the fix, feel free to add my ACK as well. Has it been tested on > guest kernel? have not tested on guest kernel.Will do. ~Pratyush > > Acked-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com> > > > > On Mon, Aug 22, 2016 at 8:35 PM, Masami Hiramatsu <mhiramat@kernel.org> > wrote: > > > On Mon, 22 Aug 2016 12:16:00 +0530 > > Pratyush Anand <panand@redhat.com> wrote: > > > > > Whenever we are hitting a kprobe from a none-kprobe debug exception > > > handler, we hit an infinite occurrences of "Unexpected kernel single-step > > > exception at EL1" > > > > > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > > > exception mode. When it is set then Watchpoint, Breakpoint, and Software > > > Step exceptions are masked. However, software Breakpoint Instruction > > > exceptions can never be masked. Therefore, if we ever execute a BRK > > > instruction, irrespective of D-bit setting, we will be receiving a > > > corresponding breakpoint exception. > > > For example: > > > - We are executing kprobe pre/post handler, and kprobe has been inserted > > in > > > one of the instruction of a function called by handler. So, it executes > > > BRK instruction and we land into the case of KPROBE_REENTER. (This > > case is > > > already handled by current code) > > > - We are executing uprobe handler or any other BRK handler such as in > > > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > > > enter into kprobe breakpoint handler,from another BRK handler.(This > > case > > > is not being handled currently) > > > In all such cases kprobe breakpoint exception will be raised when we were > > > already in debug exception mode. > > > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > > > exception was taken.So, in above example cases we would find it set in > > > kprobe breakpoint handler. > > > Single step exception will always be followed by a kprobe breakpoint > > > exception.However, it will only be raised gracefully if we clear D bit > > > while returning from breakpoint exception. If D bit is set then, it > > results > > > into undefined exception and when it's handler enables dbg then single > > step > > > exception is generated, however it will never be handled(because address > > > does not match and therefore treated as unexpected). > > > This patch clears D-flag unconditionally in setup_singlestep, so that we > > > can always get single step exception correctly after returning from > > > breakpoint exception. > > > Additionally, it also removes D-flag set statement for KPROBE_REENTER > > > return path, because debug exception for KPROBE_REENTER will always take > > > place in a debug exception state. So, D-flag will already be set in this > > > case. > > > > OK, It explains this issue clearly. > > > > > > > > Signed-off-by: Pratyush Anand <panand@redhat.com> > > > > Acked-by: Masami Hiramatsu <mhiramat@kernel.org> > > > > Thanks! > > > > > --- > > > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > > > 1 file changed, 13 insertions(+), 16 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/probes/kprobes.c > > b/arch/arm64/kernel/probes/kprobes.c > > > index e51f4ad97525..6e13361b9c01 100644 > > > --- a/arch/arm64/kernel/probes/kprobes.c > > > +++ b/arch/arm64/kernel/probes/kprobes.c > > > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct > > kprobe *p) > > > } > > > > > > /* > > > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > > > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > > > - * probe i.e. when probe hit from kprobe handler context upon > > > - * executing the pre/post handlers. In this case we return with > > > - * D-flag clear so that single-stepping can be carried-out. > > > - * > > > - * Leave D-flag set in all other cases. > > > + * When PSTATE.D is set (masked), then software step exceptions can not > > be > > > + * generated. > > > + * SPSR's D bit shows the value of PSTATE.D immediately before the > > > + * exception was taken. PSTATE.D is set while entering into any > > exception > > > + * mode, however software clears it for any normal > > (none-debug-exception) > > > + * mode in the exception entry. Therefore, when we are entering into > > kprobe > > > + * breakpoint handler from any normal mode then SPSR.D bit is already > > > + * cleared, however it is set when we are entering from any debug > > exception > > > + * mode. > > > + * Since we always need to generate single step exception after a kprobe > > > + * breakpoint exception therefore we need to clear it unconditionally, > > when > > > + * we become sure that the current breakpoint exception is for kprobe. > > > */ > > > static void __kprobes > > > spsr_set_debug_flag(struct pt_regs *regs, int mask) > > > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct > > kprobe *p, > > > > > > set_ss_context(kcb, slot); /* mark pending ss */ > > > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 0); > > > - else > > > - WARN_ON(regs->pstate & PSR_D_BIT); > > > + spsr_set_debug_flag(regs, 0); > > > > > > /* IRQs and single stepping do not mix well. */ > > > kprobes_save_local_irqflag(kcb, regs); > > > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs > > *regs, unsigned int fsr) > > > BUG(); > > > > > > kernel_disable_single_step(); > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 1); > > > > > > if (kcb->kprobe_status == KPROBE_REENTER) > > > restore_previous_kprobe(kcb); > > > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, > > unsigned int esr) > > > kprobes_restore_local_irqflag(kcb, regs); > > > kernel_disable_single_step(); > > > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 1); > > > - > > > post_kprobe_handler(kcb, regs); > > > } > > > > > > -- > > > 2.5.5 > > > > > > > > > -- > > Masami Hiramatsu <mhiramat@kernel.org> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-08-24 10:06 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-24 10:06 UTC (permalink / raw) To: Sandeepa Prabhu Cc: Masami Hiramatsu, Will Deacon, Catalin Marinas, Dave Long, linux-arm-kernel, William Cohen, open list, Marc Zyngier Hi Sandeepa, Thanks for the review. On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: > Thanks for the fix, feel free to add my ACK as well. Has it been tested on > guest kernel? have not tested on guest kernel.Will do. ~Pratyush > > Acked-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com> > > > > On Mon, Aug 22, 2016 at 8:35 PM, Masami Hiramatsu <mhiramat@kernel.org> > wrote: > > > On Mon, 22 Aug 2016 12:16:00 +0530 > > Pratyush Anand <panand@redhat.com> wrote: > > > > > Whenever we are hitting a kprobe from a none-kprobe debug exception > > > handler, we hit an infinite occurrences of "Unexpected kernel single-step > > > exception at EL1" > > > > > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > > > exception mode. When it is set then Watchpoint, Breakpoint, and Software > > > Step exceptions are masked. However, software Breakpoint Instruction > > > exceptions can never be masked. Therefore, if we ever execute a BRK > > > instruction, irrespective of D-bit setting, we will be receiving a > > > corresponding breakpoint exception. > > > For example: > > > - We are executing kprobe pre/post handler, and kprobe has been inserted > > in > > > one of the instruction of a function called by handler. So, it executes > > > BRK instruction and we land into the case of KPROBE_REENTER. (This > > case is > > > already handled by current code) > > > - We are executing uprobe handler or any other BRK handler such as in > > > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > > > enter into kprobe breakpoint handler,from another BRK handler.(This > > case > > > is not being handled currently) > > > In all such cases kprobe breakpoint exception will be raised when we were > > > already in debug exception mode. > > > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > > > exception was taken.So, in above example cases we would find it set in > > > kprobe breakpoint handler. > > > Single step exception will always be followed by a kprobe breakpoint > > > exception.However, it will only be raised gracefully if we clear D bit > > > while returning from breakpoint exception. If D bit is set then, it > > results > > > into undefined exception and when it's handler enables dbg then single > > step > > > exception is generated, however it will never be handled(because address > > > does not match and therefore treated as unexpected). > > > This patch clears D-flag unconditionally in setup_singlestep, so that we > > > can always get single step exception correctly after returning from > > > breakpoint exception. > > > Additionally, it also removes D-flag set statement for KPROBE_REENTER > > > return path, because debug exception for KPROBE_REENTER will always take > > > place in a debug exception state. So, D-flag will already be set in this > > > case. > > > > OK, It explains this issue clearly. > > > > > > > > Signed-off-by: Pratyush Anand <panand@redhat.com> > > > > Acked-by: Masami Hiramatsu <mhiramat@kernel.org> > > > > Thanks! > > > > > --- > > > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > > > 1 file changed, 13 insertions(+), 16 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/probes/kprobes.c > > b/arch/arm64/kernel/probes/kprobes.c > > > index e51f4ad97525..6e13361b9c01 100644 > > > --- a/arch/arm64/kernel/probes/kprobes.c > > > +++ b/arch/arm64/kernel/probes/kprobes.c > > > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct > > kprobe *p) > > > } > > > > > > /* > > > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > > > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > > > - * probe i.e. when probe hit from kprobe handler context upon > > > - * executing the pre/post handlers. In this case we return with > > > - * D-flag clear so that single-stepping can be carried-out. > > > - * > > > - * Leave D-flag set in all other cases. > > > + * When PSTATE.D is set (masked), then software step exceptions can not > > be > > > + * generated. > > > + * SPSR's D bit shows the value of PSTATE.D immediately before the > > > + * exception was taken. PSTATE.D is set while entering into any > > exception > > > + * mode, however software clears it for any normal > > (none-debug-exception) > > > + * mode in the exception entry. Therefore, when we are entering into > > kprobe > > > + * breakpoint handler from any normal mode then SPSR.D bit is already > > > + * cleared, however it is set when we are entering from any debug > > exception > > > + * mode. > > > + * Since we always need to generate single step exception after a kprobe > > > + * breakpoint exception therefore we need to clear it unconditionally, > > when > > > + * we become sure that the current breakpoint exception is for kprobe. > > > */ > > > static void __kprobes > > > spsr_set_debug_flag(struct pt_regs *regs, int mask) > > > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct > > kprobe *p, > > > > > > set_ss_context(kcb, slot); /* mark pending ss */ > > > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 0); > > > - else > > > - WARN_ON(regs->pstate & PSR_D_BIT); > > > + spsr_set_debug_flag(regs, 0); > > > > > > /* IRQs and single stepping do not mix well. */ > > > kprobes_save_local_irqflag(kcb, regs); > > > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs > > *regs, unsigned int fsr) > > > BUG(); > > > > > > kernel_disable_single_step(); > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 1); > > > > > > if (kcb->kprobe_status == KPROBE_REENTER) > > > restore_previous_kprobe(kcb); > > > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, > > unsigned int esr) > > > kprobes_restore_local_irqflag(kcb, regs); > > > kernel_disable_single_step(); > > > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > > - spsr_set_debug_flag(regs, 1); > > > - > > > post_kprobe_handler(kcb, regs); > > > } > > > > > > -- > > > 2.5.5 > > > > > > > > > -- > > Masami Hiramatsu <mhiramat@kernel.org> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler 2016-08-24 10:06 ` Pratyush Anand @ 2016-09-15 16:15 ` Pratyush Anand -1 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-09-15 16:15 UTC (permalink / raw) To: linux-arm-kernel Hi Sandeepa, On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: > Hi Sandeepa, > > Thanks for the review. > > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on >> guest kernel? > > have not tested on guest kernel.Will do. I tested it with guest kernel, and it worked there too :-). @Catalin: any more comment for this patch? If not, then may be you can take it. ~Pratyush ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-09-15 16:15 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-09-15 16:15 UTC (permalink / raw) To: Sandeepa Prabhu, Catalin Marinas Cc: Masami Hiramatsu, Will Deacon, Dave Long, linux-arm-kernel, William Cohen, open list, Marc Zyngier Hi Sandeepa, On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: > Hi Sandeepa, > > Thanks for the review. > > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on >> guest kernel? > > have not tested on guest kernel.Will do. I tested it with guest kernel, and it worked there too :-). @Catalin: any more comment for this patch? If not, then may be you can take it. ~Pratyush ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler 2016-09-15 16:15 ` Pratyush Anand @ 2016-09-15 16:18 ` Will Deacon -1 siblings, 0 replies; 14+ messages in thread From: Will Deacon @ 2016-09-15 16:18 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 15, 2016 at 09:45:09PM +0530, Pratyush Anand wrote: > On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: > > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: > >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on > >> guest kernel? > > > > have not tested on guest kernel.Will do. > > I tested it with guest kernel, and it worked there too :-). > > @Catalin: any more comment for this patch? If not, then may be you can take it. I already queued this on the arm64 for-next/core branch. Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-09-15 16:18 ` Will Deacon 0 siblings, 0 replies; 14+ messages in thread From: Will Deacon @ 2016-09-15 16:18 UTC (permalink / raw) To: Pratyush Anand Cc: Sandeepa Prabhu, Catalin Marinas, Masami Hiramatsu, Dave Long, linux-arm-kernel, William Cohen, open list, Marc Zyngier On Thu, Sep 15, 2016 at 09:45:09PM +0530, Pratyush Anand wrote: > On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: > > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: > >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on > >> guest kernel? > > > > have not tested on guest kernel.Will do. > > I tested it with guest kernel, and it worked there too :-). > > @Catalin: any more comment for this patch? If not, then may be you can take it. I already queued this on the arm64 for-next/core branch. Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler 2016-09-15 16:18 ` Will Deacon @ 2016-09-15 16:35 ` Pratyush Anand -1 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-09-15 16:35 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 15, 2016 at 9:48 PM, Will Deacon <will.deacon@arm.com> wrote: > On Thu, Sep 15, 2016 at 09:45:09PM +0530, Pratyush Anand wrote: >> On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: >> > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: >> >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on >> >> guest kernel? >> > >> > have not tested on guest kernel.Will do. >> >> I tested it with guest kernel, and it worked there too :-). >> >> @Catalin: any more comment for this patch? If not, then may be you can take it. > > I already queued this on the arm64 for-next/core branch. > Thanks Will :-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-09-15 16:35 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-09-15 16:35 UTC (permalink / raw) To: Will Deacon Cc: Sandeepa Prabhu, Catalin Marinas, Masami Hiramatsu, Dave Long, linux-arm-kernel, William Cohen, open list, Marc Zyngier On Thu, Sep 15, 2016 at 9:48 PM, Will Deacon <will.deacon@arm.com> wrote: > On Thu, Sep 15, 2016 at 09:45:09PM +0530, Pratyush Anand wrote: >> On Wed, Aug 24, 2016 at 3:36 PM, Pratyush Anand <panand@redhat.com> wrote: >> > On 23/08/2016:04:33:08 PM, Sandeepa Prabhu wrote: >> >> Thanks for the fix, feel free to add my ACK as well. Has it been tested on >> >> guest kernel? >> > >> > have not tested on guest kernel.Will do. >> >> I tested it with guest kernel, and it worked there too :-). >> >> @Catalin: any more comment for this patch? If not, then may be you can take it. > > I already queued this on the arm64 for-next/core branch. > Thanks Will :-) ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <CAAOGbpzfn3e0YnpRaWk=GTmJonfC+N8MML2H4sbdT-xSNykgxA@mail.gmail.com>]
* [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler [not found] ` <CAAOGbpzfn3e0YnpRaWk=GTmJonfC+N8MML2H4sbdT-xSNykgxA@mail.gmail.com> @ 2016-08-24 9:49 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-24 9:49 UTC (permalink / raw) To: linux-arm-kernel Hi Sandeepa, On 24/08/2016:03:09:16 PM, Sandeepa Prabhu wrote: > On Mon, Aug 22, 2016 at 12:16 PM, Pratyush Anand <panand@redhat.com> wrote: > > > Whenever we are hitting a kprobe from a none-kprobe debug exception > > handler, we hit an infinite occurrences of "Unexpected kernel single-step > > exception at EL1" > > > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > > exception mode. When it is set then Watchpoint, Breakpoint, and Software > > Step exceptions are masked. However, software Breakpoint Instruction > > exceptions can never be masked. Therefore, if we ever execute a BRK > > instruction, irrespective of D-bit setting, we will be receiving a > > corresponding breakpoint exception. > > For example: > > - We are executing kprobe pre/post handler, and kprobe has been inserted in > > one of the instruction of a function called by handler. So, it executes > > BRK instruction and we land into the case of KPROBE_REENTER. (This case > > is > > already handled by current code) > > - We are executing uprobe handler or any other BRK handler such as in > > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > > enter into kprobe breakpoint handler,from another BRK handler.(This case > > is not being handled currently) > > In all such cases kprobe breakpoint exception will be raised when we were > > already in debug exception mode. > > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > > exception was taken.So, in above example cases we would find it set in > > kprobe breakpoint handler. > > Single step exception will always be followed by a kprobe breakpoint > > exception.However, it will only be raised gracefully if we clear D bit > > while returning from breakpoint exception. If D bit is set then, it results > > into undefined exception and when it's handler enables dbg then single step > > exception is generated, however it will never be handled(because address > > does not match and therefore treated as unexpected). > > This patch clears D-flag unconditionally in setup_singlestep, so that we > > can always get single step exception correctly after returning from > > breakpoint exception. > > Additionally, it also removes D-flag set statement for KPROBE_REENTER > > return path, because debug exception for KPROBE_REENTER will always take > > place in a debug exception state. So, D-flag will already be set in this > > case. > > > > Signed-off-by: Pratyush Anand <panand@redhat.com> > > --- > > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > > 1 file changed, 13 insertions(+), 16 deletions(-) > > > > diff --git a/arch/arm64/kernel/probes/kprobes.c > > b/arch/arm64/kernel/probes/kprobes.c > > index e51f4ad97525..6e13361b9c01 100644 > > --- a/arch/arm64/kernel/probes/kprobes.c > > +++ b/arch/arm64/kernel/probes/kprobes.c > > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct > > kprobe *p) > > } > > > > /* > > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > > - * probe i.e. when probe hit from kprobe handler context upon > > - * executing the pre/post handlers. In this case we return with > > - * D-flag clear so that single-stepping can be carried-out. > > - * > > - * Leave D-flag set in all other cases. > > + * When PSTATE.D is set (masked), then software step exceptions can not be > > + * generated. > > + * SPSR's D bit shows the value of PSTATE.D immediately before the > > + * exception was taken. PSTATE.D is set while entering into any exception > > + * mode, however software clears it for any normal (none-debug-exception) > > + * mode in the exception entry. Therefore, when we are entering into > > kprobe > > + * breakpoint handler from any normal mode then SPSR.D bit is already > > + * cleared, however it is set when we are entering from any debug > > exception > > + * mode. > > + * Since we always need to generate single step exception after a kprobe > > + * breakpoint exception therefore we need to clear it unconditionally, > > when > > + * we become sure that the current breakpoint exception is for kprobe. > > */ > > static void __kprobes > > spsr_set_debug_flag(struct pt_regs *regs, int mask) > > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe > > *p, > > > > set_ss_context(kcb, slot); /* mark pending ss */ > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 0); > > - else > > - WARN_ON(regs->pstate & PSR_D_BIT); > > + spsr_set_debug_flag(regs, 0); > > > > /* IRQs and single stepping do not mix well. */ > > kprobes_save_local_irqflag(kcb, regs); > > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs > > *regs, unsigned int fsr) > > BUG(); > > > > kernel_disable_single_step(); > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 1); > > > > if (kcb->kprobe_status == KPROBE_REENTER) > > restore_previous_kprobe(kcb); > > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, > > unsigned int esr) > > kprobes_restore_local_irqflag(kcb, regs); > > kernel_disable_single_step(); > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 1); > > - > > post_kprobe_handler(kcb, regs); > > } > > > > Thanks for the fix, feel free to add my ACK as well. > Acked-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com> Thanks. > > btw, has it been tested on guest kernel? I have not tested it on guest kernel. I will setup a kvm guest to see if all works fine there too. ~Pratyush ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler @ 2016-08-24 9:49 ` Pratyush Anand 0 siblings, 0 replies; 14+ messages in thread From: Pratyush Anand @ 2016-08-24 9:49 UTC (permalink / raw) To: Sandeepa Prabhu Cc: Will Deacon, Catalin Marinas, Dave Long, linux-arm-kernel, William Cohen, Masami Hiramatsu, open list, Marc Zyngier Hi Sandeepa, On 24/08/2016:03:09:16 PM, Sandeepa Prabhu wrote: > On Mon, Aug 22, 2016 at 12:16 PM, Pratyush Anand <panand@redhat.com> wrote: > > > Whenever we are hitting a kprobe from a none-kprobe debug exception > > handler, we hit an infinite occurrences of "Unexpected kernel single-step > > exception at EL1" > > > > PSTATE.D is debug exception mask bit. It is set whenever we enter into an > > exception mode. When it is set then Watchpoint, Breakpoint, and Software > > Step exceptions are masked. However, software Breakpoint Instruction > > exceptions can never be masked. Therefore, if we ever execute a BRK > > instruction, irrespective of D-bit setting, we will be receiving a > > corresponding breakpoint exception. > > For example: > > - We are executing kprobe pre/post handler, and kprobe has been inserted in > > one of the instruction of a function called by handler. So, it executes > > BRK instruction and we land into the case of KPROBE_REENTER. (This case > > is > > already handled by current code) > > - We are executing uprobe handler or any other BRK handler such as in > > WARN_ON (BRK BUG_BRK_IMM), and we trace that path using kprobe.So, we > > enter into kprobe breakpoint handler,from another BRK handler.(This case > > is not being handled currently) > > In all such cases kprobe breakpoint exception will be raised when we were > > already in debug exception mode. > > SPSR's D bit (bit 9) shows the value of PSTATE.D immediately before the > > exception was taken.So, in above example cases we would find it set in > > kprobe breakpoint handler. > > Single step exception will always be followed by a kprobe breakpoint > > exception.However, it will only be raised gracefully if we clear D bit > > while returning from breakpoint exception. If D bit is set then, it results > > into undefined exception and when it's handler enables dbg then single step > > exception is generated, however it will never be handled(because address > > does not match and therefore treated as unexpected). > > This patch clears D-flag unconditionally in setup_singlestep, so that we > > can always get single step exception correctly after returning from > > breakpoint exception. > > Additionally, it also removes D-flag set statement for KPROBE_REENTER > > return path, because debug exception for KPROBE_REENTER will always take > > place in a debug exception state. So, D-flag will already be set in this > > case. > > > > Signed-off-by: Pratyush Anand <panand@redhat.com> > > --- > > arch/arm64/kernel/probes/kprobes.c | 29 +++++++++++++---------------- > > 1 file changed, 13 insertions(+), 16 deletions(-) > > > > diff --git a/arch/arm64/kernel/probes/kprobes.c > > b/arch/arm64/kernel/probes/kprobes.c > > index e51f4ad97525..6e13361b9c01 100644 > > --- a/arch/arm64/kernel/probes/kprobes.c > > +++ b/arch/arm64/kernel/probes/kprobes.c > > @@ -166,13 +166,18 @@ static void __kprobes set_current_kprobe(struct > > kprobe *p) > > } > > > > /* > > - * The D-flag (Debug mask) is set (masked) upon debug exception entry. > > - * Kprobes needs to clear (unmask) D-flag -ONLY- in case of recursive > > - * probe i.e. when probe hit from kprobe handler context upon > > - * executing the pre/post handlers. In this case we return with > > - * D-flag clear so that single-stepping can be carried-out. > > - * > > - * Leave D-flag set in all other cases. > > + * When PSTATE.D is set (masked), then software step exceptions can not be > > + * generated. > > + * SPSR's D bit shows the value of PSTATE.D immediately before the > > + * exception was taken. PSTATE.D is set while entering into any exception > > + * mode, however software clears it for any normal (none-debug-exception) > > + * mode in the exception entry. Therefore, when we are entering into > > kprobe > > + * breakpoint handler from any normal mode then SPSR.D bit is already > > + * cleared, however it is set when we are entering from any debug > > exception > > + * mode. > > + * Since we always need to generate single step exception after a kprobe > > + * breakpoint exception therefore we need to clear it unconditionally, > > when > > + * we become sure that the current breakpoint exception is for kprobe. > > */ > > static void __kprobes > > spsr_set_debug_flag(struct pt_regs *regs, int mask) > > @@ -245,10 +250,7 @@ static void __kprobes setup_singlestep(struct kprobe > > *p, > > > > set_ss_context(kcb, slot); /* mark pending ss */ > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 0); > > - else > > - WARN_ON(regs->pstate & PSR_D_BIT); > > + spsr_set_debug_flag(regs, 0); > > > > /* IRQs and single stepping do not mix well. */ > > kprobes_save_local_irqflag(kcb, regs); > > @@ -333,8 +335,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs > > *regs, unsigned int fsr) > > BUG(); > > > > kernel_disable_single_step(); > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 1); > > > > if (kcb->kprobe_status == KPROBE_REENTER) > > restore_previous_kprobe(kcb); > > @@ -457,9 +457,6 @@ kprobe_single_step_handler(struct pt_regs *regs, > > unsigned int esr) > > kprobes_restore_local_irqflag(kcb, regs); > > kernel_disable_single_step(); > > > > - if (kcb->kprobe_status == KPROBE_REENTER) > > - spsr_set_debug_flag(regs, 1); > > - > > post_kprobe_handler(kcb, regs); > > } > > > > Thanks for the fix, feel free to add my ACK as well. > Acked-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com> Thanks. > > btw, has it been tested on guest kernel? I have not tested it on guest kernel. I will setup a kvm guest to see if all works fine there too. ~Pratyush ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-09-15 16:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-22 6:46 [PATCH] arm64: kprobe: Always clear pstate.D in breakpoint exception handler Pratyush Anand
2016-08-22 6:46 ` Pratyush Anand
2016-08-22 15:05 ` Masami Hiramatsu
2016-08-22 15:05 ` Masami Hiramatsu
[not found] ` <CAAOGbpwcx3G8Jdf1AfNRLrkZJQLaCcLWQPRppjvpy3=hSrffjg@mail.gmail.com>
2016-08-24 10:06 ` Pratyush Anand
2016-08-24 10:06 ` Pratyush Anand
2016-09-15 16:15 ` Pratyush Anand
2016-09-15 16:15 ` Pratyush Anand
2016-09-15 16:18 ` Will Deacon
2016-09-15 16:18 ` Will Deacon
2016-09-15 16:35 ` Pratyush Anand
2016-09-15 16:35 ` Pratyush Anand
[not found] ` <CAAOGbpzfn3e0YnpRaWk=GTmJonfC+N8MML2H4sbdT-xSNykgxA@mail.gmail.com>
2016-08-24 9:49 ` Pratyush Anand
2016-08-24 9:49 ` Pratyush Anand
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.