Re: [BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Masami Hiramatsu]

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

> On Tue, 2017-02-14 at 00:04 +0900, Masami Hiramatsu wrote:
> > Kprobes/arm skips single-stepping (moreover handling the event)
> > if the conditional instruction must not be executed. This
> > also apply the rule when we hit the recursing kprobe, so
> > that kprobe does not count nmissed up in that case.
> 
> Perhaps that last sentence would read better if written something like:
> 
> "This also applies that rule when we hit a recursing kprobe, so that the
> nmissed count isn't incremented in that case."

OK, Thanks!

> 
> 
> > Signed-off-by: Masami Hiramatsu 
> 
> Acked-by: Jon Medhurst 
> 
> > ---
> >  arch/arm/probes/kprobes/core.c |   19 ++-
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> > index 264fedb..84989ae 100644
> > --- a/arch/arm/probes/kprobes/core.c
> > +++ b/arch/arm/probes/kprobes/core.c
> > @@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> >  #endif
> >  
> > if (p) {
> > -   if (cur) {
> > +   if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> > +   /*
> > +* Probe hit but conditional execution check failed,
> > +* so just skip the instruction and continue as if
> > +* nothing had happened.
> > +* In this case, we can skip recursing check too.
> > +*/
> > +   singlestep_skip(p, regs);
> > +   } else if (cur) {
> > /* Kprobe is pending, so we're recursing. */
> > switch (kcb->kprobe_status) {
> > case KPROBE_HIT_ACTIVE:
> > @@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > /* impossible cases */
> > BUG();
> > }
> > -   } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> > +   } else {
> > /* Probe hit and conditional execution check ok. */
> > set_current_kprobe(p);
> > kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > @@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > }
> > reset_current_kprobe();
> > }
> > -   } else {
> > -   /*
> > -* Probe hit but conditional execution check failed,
> > -* so just skip the instruction and continue as if
> > -* nothing had happened.
> > -*/
> > -   singlestep_skip(p, regs);
> > }
> > } else if (cur) {
> > /* We probably hit a jprobe.  Call its break handler. */
> > 


-- 
Masami Hiramatsu 

Re: [BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Masami Hiramatsu]

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

> On Tue, 2017-02-14 at 00:04 +0900, Masami Hiramatsu wrote:
> > Kprobes/arm skips single-stepping (moreover handling the event)
> > if the conditional instruction must not be executed. This
> > also apply the rule when we hit the recursing kprobe, so
> > that kprobe does not count nmissed up in that case.
> 
> Perhaps that last sentence would read better if written something like:
> 
> "This also applies that rule when we hit a recursing kprobe, so that the
> nmissed count isn't incremented in that case."

OK, Thanks!

> 
> 
> > Signed-off-by: Masami Hiramatsu 
> 
> Acked-by: Jon Medhurst 
> 
> > ---
> >  arch/arm/probes/kprobes/core.c |   19 ++-
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> > index 264fedb..84989ae 100644
> > --- a/arch/arm/probes/kprobes/core.c
> > +++ b/arch/arm/probes/kprobes/core.c
> > @@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> >  #endif
> >  
> > if (p) {
> > -   if (cur) {
> > +   if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> > +   /*
> > +* Probe hit but conditional execution check failed,
> > +* so just skip the instruction and continue as if
> > +* nothing had happened.
> > +* In this case, we can skip recursing check too.
> > +*/
> > +   singlestep_skip(p, regs);
> > +   } else if (cur) {
> > /* Kprobe is pending, so we're recursing. */
> > switch (kcb->kprobe_status) {
> > case KPROBE_HIT_ACTIVE:
> > @@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > /* impossible cases */
> > BUG();
> > }
> > -   } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> > +   } else {
> > /* Probe hit and conditional execution check ok. */
> > set_current_kprobe(p);
> > kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > @@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
> > }
> > reset_current_kprobe();
> > }
> > -   } else {
> > -   /*
> > -* Probe hit but conditional execution check failed,
> > -* so just skip the instruction and continue as if
> > -* nothing had happened.
> > -*/
> > -   singlestep_skip(p, regs);
> > }
> > } else if (cur) {
> > /* We probably hit a jprobe.  Call its break handler. */
> > 


-- 
Masami Hiramatsu 

Re: [BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Jon Medhurst (Tixy)]

On Tue, 2017-02-14 at 00:04 +0900, Masami Hiramatsu wrote:
> Kprobes/arm skips single-stepping (moreover handling the event)
> if the conditional instruction must not be executed. This
> also apply the rule when we hit the recursing kprobe, so
> that kprobe does not count nmissed up in that case.

Perhaps that last sentence would read better if written something like:

"This also applies that rule when we hit a recursing kprobe, so that the
nmissed count isn't incremented in that case."


> Signed-off-by: Masami Hiramatsu 

Acked-by: Jon Medhurst 

> ---
>  arch/arm/probes/kprobes/core.c |   19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 264fedb..84989ae 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>  #endif
>  
>   if (p) {
> - if (cur) {
> + if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> + /*
> +  * Probe hit but conditional execution check failed,
> +  * so just skip the instruction and continue as if
> +  * nothing had happened.
> +  * In this case, we can skip recursing check too.
> +  */
> + singlestep_skip(p, regs);
> + } else if (cur) {
>   /* Kprobe is pending, so we're recursing. */
>   switch (kcb->kprobe_status) {
>   case KPROBE_HIT_ACTIVE:
> @@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   /* impossible cases */
>   BUG();
>   }
> - } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> + } else {
>   /* Probe hit and conditional execution check ok. */
>   set_current_kprobe(p);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> @@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   }
>   reset_current_kprobe();
>   }
> - } else {
> - /*
> -  * Probe hit but conditional execution check failed,
> -  * so just skip the instruction and continue as if
> -  * nothing had happened.
> -  */
> - singlestep_skip(p, regs);
>   }
>   } else if (cur) {
>   /* We probably hit a jprobe.  Call its break handler. */
> 

Re: [BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Jon Medhurst (Tixy)]

On Tue, 2017-02-14 at 00:04 +0900, Masami Hiramatsu wrote:
> Kprobes/arm skips single-stepping (moreover handling the event)
> if the conditional instruction must not be executed. This
> also apply the rule when we hit the recursing kprobe, so
> that kprobe does not count nmissed up in that case.

Perhaps that last sentence would read better if written something like:

"This also applies that rule when we hit a recursing kprobe, so that the
nmissed count isn't incremented in that case."


> Signed-off-by: Masami Hiramatsu 

Acked-by: Jon Medhurst 

> ---
>  arch/arm/probes/kprobes/core.c |   19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 264fedb..84989ae 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>  #endif
>  
>   if (p) {
> - if (cur) {
> + if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> + /*
> +  * Probe hit but conditional execution check failed,
> +  * so just skip the instruction and continue as if
> +  * nothing had happened.
> +  * In this case, we can skip recursing check too.
> +  */
> + singlestep_skip(p, regs);
> + } else if (cur) {
>   /* Kprobe is pending, so we're recursing. */
>   switch (kcb->kprobe_status) {
>   case KPROBE_HIT_ACTIVE:
> @@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   /* impossible cases */
>   BUG();
>   }
> - } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
> + } else {
>   /* Probe hit and conditional execution check ok. */
>   set_current_kprobe(p);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> @@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
>   }
>   reset_current_kprobe();
>   }
> - } else {
> - /*
> -  * Probe hit but conditional execution check failed,
> -  * so just skip the instruction and continue as if
> -  * nothing had happened.
> -  */
> - singlestep_skip(p, regs);
>   }
>   } else if (cur) {
>   /* We probably hit a jprobe.  Call its break handler. */
> 

[BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Masami Hiramatsu]

Kprobes/arm skips single-stepping (moreover handling the event)
if the conditional instruction must not be executed. This
also apply the rule when we hit the recursing kprobe, so
that kprobe does not count nmissed up in that case.

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

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 264fedb..84989ae 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
 #endif
 
if (p) {
-   if (cur) {
+   if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
+   /*
+* Probe hit but conditional execution check failed,
+* so just skip the instruction and continue as if
+* nothing had happened.
+* In this case, we can skip recursing check too.
+*/
+   singlestep_skip(p, regs);
+   } else if (cur) {
/* Kprobe is pending, so we're recursing. */
switch (kcb->kprobe_status) {
case KPROBE_HIT_ACTIVE:
@@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
/* impossible cases */
BUG();
}
-   } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
+   } else {
/* Probe hit and conditional execution check ok. */
set_current_kprobe(p);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
@@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
}
reset_current_kprobe();
}
-   } else {
-   /*
-* Probe hit but conditional execution check failed,
-* so just skip the instruction and continue as if
-* nothing had happened.
-*/
-   singlestep_skip(p, regs);
}
} else if (cur) {
/* We probably hit a jprobe.  Call its break handler. */

[BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible

[Masami Hiramatsu]

Kprobes/arm skips single-stepping (moreover handling the event)
if the conditional instruction must not be executed. This
also apply the rule when we hit the recursing kprobe, so
that kprobe does not count nmissed up in that case.

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

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 264fedb..84989ae 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -265,7 +265,15 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
 #endif
 
if (p) {
-   if (cur) {
+   if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
+   /*
+* Probe hit but conditional execution check failed,
+* so just skip the instruction and continue as if
+* nothing had happened.
+* In this case, we can skip recursing check too.
+*/
+   singlestep_skip(p, regs);
+   } else if (cur) {
/* Kprobe is pending, so we're recursing. */
switch (kcb->kprobe_status) {
case KPROBE_HIT_ACTIVE:
@@ -288,7 +296,7 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
/* impossible cases */
BUG();
}
-   } else if (p->ainsn.insn_check_cc(regs->ARM_cpsr)) {
+   } else {
/* Probe hit and conditional execution check ok. */
set_current_kprobe(p);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
@@ -309,13 +317,6 @@ void __kprobes kprobe_handler(struct pt_regs *regs)
}
reset_current_kprobe();
}
-   } else {
-   /*
-* Probe hit but conditional execution check failed,
-* so just skip the instruction and continue as if
-* nothing had happened.
-*/
-   singlestep_skip(p, regs);
}
} else if (cur) {
/* We probably hit a jprobe.  Call its break handler. */