2006-03-21 10:02:39

by bibo,mao

[permalink] [raw]
Subject: [PATCH] kretprobe spinlock recursive remove

In recent linux kernel version, kretprobe in IA32 is implemented in
kretprobe_trampoline. And break trap code is removed from
retprobe_trampoline, instead trampoline_handler is called directly.
Currently if kretprobe hander hit one trap which causes another
kretprobe, there will be SPINLOCK recursive bug. This patch fixes this,
and will skip trap during kretprobe handler execution. This patch is
based on 2.6.16-rc6-mm2.

--- arch/i386/kernel/kprobes.c.bak 2006-03-21 10:35:34.000000000 +0800
+++ arch/i386/kernel/kprobes.c 2006-03-21 10:37:44.000000000 +0800
@@ -390,8 +390,11 @@ fastcall void *__kprobes trampoline_hand
/* another task is sharing our hash bucket */
continue;

- if (ri->rp && ri->rp->handler)
+ if (ri->rp && ri->rp->handler){
+ __get_cpu_var(current_kprobe) = &ri->rp->kp;
ri->rp->handler(ri, regs);
+ __get_cpu_var(current_kprobe) = NULL;
+ }

orig_ret_address = (unsigned long)ri->ret_addr;
recycle_rp_inst(ri);


2006-03-21 10:17:44

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] kretprobe spinlock recursive remove

"bibo,mao" <[email protected]> wrote:
>
> In recent linux kernel version, kretprobe in IA32 is implemented in
> kretprobe_trampoline. And break trap code is removed from
> retprobe_trampoline, instead trampoline_handler is called directly.
> Currently if kretprobe hander hit one trap which causes another
> kretprobe, there will be SPINLOCK recursive bug. This patch fixes this,
> and will skip trap during kretprobe handler execution. This patch is
> based on 2.6.16-rc6-mm2.

What is "recent linux kernel"? 2.6.16? This patch does apply to 2.6.16 so
I assume that's what you were referring to?

If you're referring to the kretprobes patches in -mm then which one
introduced the problem?

If you're referring to 2.6.16 then is this problem sufficiently serious to
warrant inclusion of this patch in 2.6.16.1?

Please remember to include Signed-off-by: tags.

> --- arch/i386/kernel/kprobes.c.bak 2006-03-21 10:35:34.000000000 +0800
> +++ arch/i386/kernel/kprobes.c 2006-03-21 10:37:44.000000000 +0800

Please prepare patches in `patch -p1' form.

> @@ -390,8 +390,11 @@ fastcall void *__kprobes trampoline_hand
> /* another task is sharing our hash bucket */
> continue;
>
> - if (ri->rp && ri->rp->handler)
> + if (ri->rp && ri->rp->handler){
> + __get_cpu_var(current_kprobe) = &ri->rp->kp;
> ri->rp->handler(ri, regs);
> + __get_cpu_var(current_kprobe) = NULL;
> + }
>
> orig_ret_address = (unsigned long)ri->ret_addr;
> recycle_rp_inst(ri);

Your email client appears to be altering the patches in some manner. It
required `patch -l' to make this apply.

2006-03-21 11:29:20

by bibo,mao

[permalink] [raw]
Subject: Re: [PATCH] kretprobe spinlock recursive remove

Andrew Morton wrote:
> "bibo,mao" <[email protected]> wrote:
>> In recent linux kernel version, kretprobe in IA32 is implemented in
>> kretprobe_trampoline. And break trap code is removed from
>> retprobe_trampoline, instead trampoline_handler is called directly.
>> Currently if kretprobe hander hit one trap which causes another
>> kretprobe, there will be SPINLOCK recursive bug. This patch fixes this,
>> and will skip trap during kretprobe handler execution. This patch is
>> based on 2.6.16-rc6-mm2.
>
> What is "recent linux kernel"? 2.6.16? This patch does apply to 2.6.16 so
> I assume that's what you were referring to?
>
> If you're referring to the kretprobes patches in -mm then which one
> introduced the problem?
Sorry, I did not clarify it out, it is againt
kretprobe-kretprobe-booster.patch in -mm tree.

>
> If you're referring to 2.6.16 then is this problem sufficiently serious to
> warrant inclusion of this patch in 2.6.16.1?
>
> Please remember to include Signed-off-by: tags.
>
>> --- arch/i386/kernel/kprobes.c.bak 2006-03-21 10:35:34.000000000 +0800
>> +++ arch/i386/kernel/kprobes.c 2006-03-21 10:37:44.000000000 +0800
>
> Please prepare patches in `patch -p1' form.
>
>> @@ -390,8 +390,11 @@ fastcall void *__kprobes trampoline_hand
>> /* another task is sharing our hash bucket */
>> continue;
>>
>> - if (ri->rp && ri->rp->handler)
>> + if (ri->rp && ri->rp->handler){
>> + __get_cpu_var(current_kprobe) = &ri->rp->kp;
>> ri->rp->handler(ri, regs);
>> + __get_cpu_var(current_kprobe) = NULL;
>> + }
>>
>> orig_ret_address = (unsigned long)ri->ret_addr;
>> recycle_rp_inst(ri);
>
> Your email client appears to be altering the patches in some manner. It
> required `patch -l' to make this apply.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>