Re: [BUGFIX][PATCH -tip] x86: kretprobe-booster interrupt emulation code fix

Masami Hiramatsu
2009-03-23T20:48:00Z

Bharata B Rao wrote:
> On Mon, Mar 23, 2009 at 10:14:52AM -0400, Masami Hiramatsu wrote:
>> Fix interrupt emulation code in kretprobe-booster according to
>> pt_regs update (es/ds change and gs adding).
>>
>> This issue has been reported on systemtap-bugzilla:
>> http://sources.redhat.com/bugzilla/show_bug.cgi?id=9965
>
> Do you want to put some of the details from the bugzilla entry
> to this patch description so that one is not forced to look
> at the bugzilla in future when git log is done ?

Oh, sure. I picked up related part of this bug.

---
On a -tip kernel on x86_32, kretprobe_example (from samples) triggers the
following backtrace when its retprobing a class of functions that cause a
copy_from/to_user().

BUG: sleeping function called from invalid context at mm/memory.c:3196
in_atomic(): 0, irqs_disabled(): 1, pid: 2286, name: cat
1 lock held by cat/2286:
#0: (&p->lock){+.+.+.}, at: [<c04b4eb1>] seq_read+0x35/0x31d
irq event stamp: 1613
hardirqs last enabled at (1613): [<c06b5914>] _spin_unlock_irqrestore+0x3c/0x48
hardirqs last disabled at (1612): [<c06b5a7f>] _spin_lock_irqsave+0x1a/0x3f
softirqs last enabled at (1610): [<c04348c5>] __do_softirq+0x164/0x183
softirqs last disabled at (1603): [<c0404d2c>] do_softirq+0x68/0xc8
Pid: 2286, comm: cat Not tainted 2.6.29-rc8-tip-acde #1
Call Trace:
[<c0429017>] __might_sleep+0xde/0xe3
[<c048c6e1>] might_fault+0x1f/0x80
[<c0535b87>] copy_to_user+0x2f/0x106
[<c04b5120>] seq_read+0x2a4/0x31d
[<c04d4882>] proc_reg_read+0x6a/0x84
[<c04b4e7c>] ? seq_read+0x0/0x31d
[<c04d4882>] ? proc_reg_read+0x6a/0x84
[<c04d4818>] ? proc_reg_read+0x0/0x84
[<c04a1f73>] vfs_read+0x90/0xef
[<c04a208b>] sys_read+0x4e/0x75
[<c044d338>] ? trace_hardirqs_on_caller+0x11d/0x141
[<c0402fc4>] sysenter_do_call+0x12/0x38
[<c0402fc4>] ? sysenter_do_call+0x12/0x38

Steps to recreate:
1. put kretprobe on meminfo_proc_show.
2. cat /proc/meminfo
3. Your dmesg should have the above trace.

Problem doesn't happen with 2.6.29-rc8.

This is a kretprobe booster bug on x86_32. Commit ccbeed3a modifies the
pt_regs, the kretprobe_trampoline() part of the kretprobe booster needs to be
updated to handle the gs register.

---

Thank you,

>
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> ---
>> arch/x86/kernel/kprobes.c | 17 +++++++++--------
>> 1 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
>> index 55b9461..64dba72 100644
>> --- a/arch/x86/kernel/kprobes.c
>> +++ b/arch/x86/kernel/kprobes.c
>> @@ -638,13 +638,13 @@ static void __used __kprobes kretprobe_trampoline_holder(void)
>> #else
>> " pushf\n"
>> /*
>> - * Skip cs, ip, orig_ax.
>> + * Skip cs, ip, orig_ax and gs.
>> * trampoline_handler() will plug in these values
>> */
>> - " subl $12, %esp\n"
>> + " subl $16, %esp\n"
>> " pushl %fs\n"
>> - " pushl %ds\n"
>> " pushl %es\n"
>> + " pushl %ds\n"
>> " pushl %eax\n"
>> " pushl %ebp\n"
>> " pushl %edi\n"
>> @@ -655,10 +655,10 @@ static void __used __kprobes kretprobe_trampoline_holder(void)
>> " movl %esp, %eax\n"
>> " call trampoline_handler\n"
>> /* Move flags to cs */
>> - " movl 52(%esp), %edx\n"
>> - " movl %edx, 48(%esp)\n"
>> + " movl 56(%esp), %edx\n"
>> + " movl %edx, 52(%esp)\n"
>> /* Replace saved flags with true return address. */
>> - " movl %eax, 52(%esp)\n"
>> + " movl %eax, 56(%esp)\n"
>> " popl %ebx\n"
>> " popl %ecx\n"
>> " popl %edx\n"
>> @@ -666,8 +666,8 @@ static void __used __kprobes kretprobe_trampoline_holder(void)
>> " popl %edi\n"
>> " popl %ebp\n"
>> " popl %eax\n"
>> - /* Skip ip, orig_ax, es, ds, fs */
>> - " addl $20, %esp\n"
>> + /* Skip ds, es, fs, gs, orig_ax and ip */
>> + " addl $24, %esp\n"
>> " popf\n"
>> #endif
>> " ret\n");
>> @@ -694,6 +694,7 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
>> #endif
>> regs->ip = trampoline_address;
>> regs->orig_ax = ~0UL;
>> + regs->gs = 0;
>>
>> /*
>> * It is possible to have multiple instances associated with a given
>>
>
> This change works for me. I no longer see "BUG: sleeping from invalid context"
> messages with kretprobe after this change.
>
> Regards,
> Bharata.

--
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com