Re: [BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Masami Hiramatsu]

On Tue, 14 Feb 2017 10:01:43 +
"Jon Medhurst (Tixy)"  wrote:

> On Tue, 2017-02-14 at 00:03 +0900, Masami Hiramatsu wrote:
> > This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
> > handle reentered kprobe on single-stepping")
> > 
> > Since the FIQ handlers can interrupt in the single stepping
> > (or preparing the single stepping, do_debug etc.), we should
> > consider a kprobe is hit in the NMI handler. Even in that
> > case, the kprobe is allowed to be reentered as same as the
> > kprobes hit in kprobe handlers
> > (KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).
> > 
> > The real issue will happen when a kprobe hit while another
> 
> Could to with 'is' being inserted above  ^^^
> (I know this is a copy of the x86 commit message)

Ah! yes, it should be corrected. 
> 
> > reentered kprobe is processing (KPROBE_REENTER), because
> > we already consumed a saved-area for the previous kprobe.
> > 
> > Signed-off-by: Masami Hiramatsu 
> > ---
> 
> Acked-by: Jon Medhurst 

Thank you!

> 
> >  arch/arm/probes/kprobes/core.c |6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> > index a4ec240..264fedb 100644
> > --- a/arch/arm/probes/kprobes/core.c
> > +++ b/arch/arm/probes/kprobes/core.c
> > @@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > switch (kcb->kprobe_status) {
> > case KPROBE_HIT_ACTIVE:
> > case KPROBE_HIT_SSDONE:
> > +   case KPROBE_HIT_SS:
> > /* A pre- or post-handler probe got us here. */
> > kprobes_inc_nmissed_count(p);
> > save_previous_kprobe(kcb);
> > @@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > singlestep(p, regs, kcb);
> > restore_previous_kprobe(kcb);
> > break;
> > +   case KPROBE_REENTER:
> > +   /* A nested probe was hit in FIQ, it is a BUG */
> > +   pr_warn("Unrecoverable kprobe detected at 
> > %p.\n",
> > +   p->addr);
> > +   /* fall through */
> > default:
> > /* impossible cases */
> > BUG();
> > 


-- 
Masami Hiramatsu 

Re: [BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Masami Hiramatsu]

On Tue, 14 Feb 2017 10:01:43 +
"Jon Medhurst (Tixy)"  wrote:

> On Tue, 2017-02-14 at 00:03 +0900, Masami Hiramatsu wrote:
> > This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
> > handle reentered kprobe on single-stepping")
> > 
> > Since the FIQ handlers can interrupt in the single stepping
> > (or preparing the single stepping, do_debug etc.), we should
> > consider a kprobe is hit in the NMI handler. Even in that
> > case, the kprobe is allowed to be reentered as same as the
> > kprobes hit in kprobe handlers
> > (KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).
> > 
> > The real issue will happen when a kprobe hit while another
> 
> Could to with 'is' being inserted above  ^^^
> (I know this is a copy of the x86 commit message)

Ah! yes, it should be corrected. 
> 
> > reentered kprobe is processing (KPROBE_REENTER), because
> > we already consumed a saved-area for the previous kprobe.
> > 
> > Signed-off-by: Masami Hiramatsu 
> > ---
> 
> Acked-by: Jon Medhurst 

Thank you!

> 
> >  arch/arm/probes/kprobes/core.c |6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> > index a4ec240..264fedb 100644
> > --- a/arch/arm/probes/kprobes/core.c
> > +++ b/arch/arm/probes/kprobes/core.c
> > @@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > switch (kcb->kprobe_status) {
> > case KPROBE_HIT_ACTIVE:
> > case KPROBE_HIT_SSDONE:
> > +   case KPROBE_HIT_SS:
> > /* A pre- or post-handler probe got us here. */
> > kprobes_inc_nmissed_count(p);
> > save_previous_kprobe(kcb);
> > @@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > singlestep(p, regs, kcb);
> > restore_previous_kprobe(kcb);
> > break;
> > +   case KPROBE_REENTER:
> > +   /* A nested probe was hit in FIQ, it is a BUG */
> > +   pr_warn("Unrecoverable kprobe detected at 
> > %p.\n",
> > +   p->addr);
> > +   /* fall through */
> > default:
> > /* impossible cases */
> > BUG();
> > 


-- 
Masami Hiramatsu 

Re: [BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Jon Medhurst (Tixy)]

On Tue, 2017-02-14 at 00:03 +0900, Masami Hiramatsu wrote:
> This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
> handle reentered kprobe on single-stepping")
> 
> Since the FIQ handlers can interrupt in the single stepping
> (or preparing the single stepping, do_debug etc.), we should
> consider a kprobe is hit in the NMI handler. Even in that
> case, the kprobe is allowed to be reentered as same as the
> kprobes hit in kprobe handlers
> (KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).
> 
> The real issue will happen when a kprobe hit while another

Could to with 'is' being inserted above  ^^^
(I know this is a copy of the x86 commit message)

> reentered kprobe is processing (KPROBE_REENTER), because
> we already consumed a saved-area for the previous kprobe.
> 
> Signed-off-by: Masami Hiramatsu 
> ---

Acked-by: Jon Medhurst 

>  arch/arm/probes/kprobes/core.c |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240..264fedb 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   switch (kcb->kprobe_status) {
>   case KPROBE_HIT_ACTIVE:
>   case KPROBE_HIT_SSDONE:
> + case KPROBE_HIT_SS:
>   /* A pre- or post-handler probe got us here. */
>   kprobes_inc_nmissed_count(p);
>   save_previous_kprobe(kcb);
> @@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   singlestep(p, regs, kcb);
>   restore_previous_kprobe(kcb);
>   break;
> + case KPROBE_REENTER:
> + /* A nested probe was hit in FIQ, it is a BUG */
> + pr_warn("Unrecoverable kprobe detected at 
> %p.\n",
> + p->addr);
> + /* fall through */
>   default:
>   /* impossible cases */
>   BUG();
> 

Re: [BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Jon Medhurst (Tixy)]

On Tue, 2017-02-14 at 00:03 +0900, Masami Hiramatsu wrote:
> This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
> handle reentered kprobe on single-stepping")
> 
> Since the FIQ handlers can interrupt in the single stepping
> (or preparing the single stepping, do_debug etc.), we should
> consider a kprobe is hit in the NMI handler. Even in that
> case, the kprobe is allowed to be reentered as same as the
> kprobes hit in kprobe handlers
> (KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).
> 
> The real issue will happen when a kprobe hit while another

Could to with 'is' being inserted above  ^^^
(I know this is a copy of the x86 commit message)

> reentered kprobe is processing (KPROBE_REENTER), because
> we already consumed a saved-area for the previous kprobe.
> 
> Signed-off-by: Masami Hiramatsu 
> ---

Acked-by: Jon Medhurst 

>  arch/arm/probes/kprobes/core.c |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index a4ec240..264fedb 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   switch (kcb->kprobe_status) {
>   case KPROBE_HIT_ACTIVE:
>   case KPROBE_HIT_SSDONE:
> + case KPROBE_HIT_SS:
>   /* A pre- or post-handler probe got us here. */
>   kprobes_inc_nmissed_count(p);
>   save_previous_kprobe(kcb);
> @@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   singlestep(p, regs, kcb);
>   restore_previous_kprobe(kcb);
>   break;
> + case KPROBE_REENTER:
> + /* A nested probe was hit in FIQ, it is a BUG */
> + pr_warn("Unrecoverable kprobe detected at 
> %p.\n",
> + p->addr);
> + /* fall through */
>   default:
>   /* impossible cases */
>   BUG();
> 

[BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Masami Hiramatsu]

This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
handle reentered kprobe on single-stepping")

Since the FIQ handlers can interrupt in the single stepping
(or preparing the single stepping, do_debug etc.), we should
consider a kprobe is hit in the NMI handler. Even in that
case, the kprobe is allowed to be reentered as same as the
kprobes hit in kprobe handlers
(KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).

The real issue will happen when a kprobe hit while another
reentered kprobe is processing (KPROBE_REENTER), because
we already consumed a saved-area for the previous kprobe.

Signed-off-by: Masami Hiramatsu 
---
 arch/arm/probes/kprobes/core.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index a4ec240..264fedb 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
switch (kcb->kprobe_status) {
case KPROBE_HIT_ACTIVE:
case KPROBE_HIT_SSDONE:
+   case KPROBE_HIT_SS:
/* A pre- or post-handler probe got us here. */
kprobes_inc_nmissed_count(p);
save_previous_kprobe(kcb);
@@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
singlestep(p, regs, kcb);
restore_previous_kprobe(kcb);
break;
+   case KPROBE_REENTER:
+   /* A nested probe was hit in FIQ, it is a BUG */
+   pr_warn("Unrecoverable kprobe detected at 
%p.\n",
+   p->addr);
+   /* fall through */
default:
/* impossible cases */
BUG();

[BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping

[Masami Hiramatsu]

This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
handle reentered kprobe on single-stepping")

Since the FIQ handlers can interrupt in the single stepping
(or preparing the single stepping, do_debug etc.), we should
consider a kprobe is hit in the NMI handler. Even in that
case, the kprobe is allowed to be reentered as same as the
kprobes hit in kprobe handlers
(KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE).

The real issue will happen when a kprobe hit while another
reentered kprobe is processing (KPROBE_REENTER), because
we already consumed a saved-area for the previous kprobe.

Signed-off-by: Masami Hiramatsu 
---
 arch/arm/probes/kprobes/core.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index a4ec240..264fedb 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -270,6 +270,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
switch (kcb->kprobe_status) {
case KPROBE_HIT_ACTIVE:
case KPROBE_HIT_SSDONE:
+   case KPROBE_HIT_SS:
/* A pre- or post-handler probe got us here. */
kprobes_inc_nmissed_count(p);
save_previous_kprobe(kcb);
@@ -278,6 +279,11 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
singlestep(p, regs, kcb);
restore_previous_kprobe(kcb);
break;
+   case KPROBE_REENTER:
+   /* A nested probe was hit in FIQ, it is a BUG */
+   pr_warn("Unrecoverable kprobe detected at 
%p.\n",
+   p->addr);
+   /* fall through */
default:
/* impossible cases */
BUG();