2007-12-18 23:19:39

by Masami Hiramatsu

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

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 <[email protected]>
---
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: [email protected], [email protected]

2007-12-19 03:34:54

by Masami Hiramatsu

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

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 <[email protected]>
> ---
> 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: [email protected], [email protected]

2007-12-19 10:21:37

by Ingo Molnar

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

* Masami Hiramatsu <[email protected]> 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

2007-12-19 16:05:28

by Masami Hiramatsu

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

Hi Ingo,

Ingo Molnar wrote:
> * Masami Hiramatsu <[email protected]> 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: [email protected], [email protected]