[PATCH 1/2] x86: fix kprobe_handler reenable preemption

[PATCH 1/2] x86: fix kprobe_handler reenable preemption

[Masami Hiramatsu]

Fix a preemption bug in kprobe_handler(). It has to call preempt_enable()
before returning.
I think this is critical on preemptive kernel.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
---
 arch/x86/kernel/kprobes.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: b/arch/x86/kernel/kprobes.c
===================================================================
--- a/arch/x86/kernel/kprobes.c	2007-12-18 12:51:00.000000000 -0500
+++ b/arch/x86/kernel/kprobes.c	2007-12-18 12:53:38.000000000 -0500
@@ -467,7 +467,8 @@
 				arch_disarm_kprobe(p);
 				regs->ip = (unsigned long)p->addr;
 				reset_current_kprobe();
-				return 1;
+				ret = 1;
+				goto no_kprobe;
 #endif
 			}
 			/* We have reentered the kprobe_handler(), since
@@ -481,7 +482,7 @@
 			kprobes_inc_nmissed_count(p);
 			prepare_singlestep(p, regs);
 			kcb->kprobe_status = KPROBE_REENTER;
-			return 1;
+			ret = 1;
 		} else {
 			if (*addr != BREAKPOINT_INSTRUCTION) {
 			/* The breakpoint instruction was removed by

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com

Re: [PATCH 1/2] x86: fix kprobe_handler reenable preemption

[Masami Hiramatsu]

Hi

Masami Hiramatsu wrote:
> Fix a preemption bug in kprobe_handler(). It has to call preempt_enable()
> before returning.
> I think this is critical on preemptive kernel.

Sorry, this patch has a mistake.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
>  arch/x86/kernel/kprobes.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Index: b/arch/x86/kernel/kprobes.c
> ===================================================================
> --- a/arch/x86/kernel/kprobes.c	2007-12-18 12:51:00.000000000 -0500
> +++ b/arch/x86/kernel/kprobes.c	2007-12-18 12:53:38.000000000 -0500
> @@ -467,7 +467,8 @@
>  				arch_disarm_kprobe(p);
>  				regs->ip = (unsigned long)p->addr;
>  				reset_current_kprobe();
> -				return 1;
> +				ret = 1;
> +				goto no_kprobe;
>  #endif
>  			}
>  			/* We have reentered the kprobe_handler(), since
> @@ -481,7 +482,7 @@
>  			kprobes_inc_nmissed_count(p);
>  			prepare_singlestep(p, regs);
>  			kcb->kprobe_status = KPROBE_REENTER;
> -			return 1;
> +			ret = 1;

Here, "return 1;" is correct. After single step is done, preemption is enabled
in post_kprobe_handler().

>  		} else {
>  			if (*addr != BREAKPOINT_INSTRUCTION) {
>  			/* The breakpoint instruction was removed by
> 

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com

Re: [PATCH 1/2] x86: fix kprobe_handler reenable preemption

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@redhat.com> wrote:

> > -			return 1;
> > +			ret = 1;
> 
> Here, "return 1;" is correct. After single step is done, preemption is 
> enabled in post_kprobe_handler().

could you resend the fixed patch? (instead of me fixing it up by hand - 
which is error-prone)

also, this seems like .24 material as well - could you send the fix 
against -rc5 too?

	Ingo

Re: [PATCH 1/2] x86: fix kprobe_handler reenable preemption

[Masami Hiramatsu]

Hi Ingo,

Ingo Molnar wrote:
> * Masami Hiramatsu <mhiramat@redhat.com> wrote:
> 
>>> -			return 1;
>>> +			ret = 1;
>> Here, "return 1;" is correct. After single step is done, preemption is 
>> enabled in post_kprobe_handler().
> 
> could you resend the fixed patch? (instead of me fixing it up by hand - 
> which is error-prone)

Sure, I'll resend asap.

> 
> also, this seems like .24 material as well - could you send the fix 
> against -rc5 too?

-rc tree is fine. Actually, it was introduced by my cleanup patch.
Thanks!

> 
> 	Ingo

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com