From: "tiejun.chen" <tiejun.chen@windriver•com>
To: Kumar Gala <galak@kernel•crashing.org>,
"Walsh, Benjamin" <benjamin.walsh@windriver•com>
Cc: linuxppc-dev <linuxppc-dev@ozlabs•org>
Subject: Re: [v3 PATCH 1/1] booke/kprobe: make program exception to use one dedicated exception stack
Date: Thu, 21 Jul 2011 17:32:19 +0800 [thread overview]
Message-ID: <4E27F223.4060602@windriver.com> (raw)
In-Reply-To: <4E1FCFEF.7070702@windriver.com>
tiejun.chen wrote:
> Kumar Gala wrote:
>> On Jul 11, 2011, at 6:31 AM, Tiejun Chen wrote:
>>
>>> When kprobe these operations such as store-and-update-word for SP(r1),
>>>
>>> stwu r1, -A(r1)
>>>
>>> The program exception is triggered, and PPC always allocate an exception frame
>>> as shown as the follows:
>>>
>>> old r1 ----------
>>> ...
>>> nip
>>> gpr[2] ~ gpr[31]
>>> gpr[1] <--------- old r1 is stored.
>>> gpr[0]
>>> -------- <--------- pr_regs @offset 16 bytes
>>> padding
>>> STACK_FRAME_REGS_MARKER
>>> LR
>>> back chain
>>> new r1 ----------
>>> Then emulate_step() will emulate this instruction, 'stwu'. Actually its
>>> equivalent to:
>>> 1> Update pr_regs->gpr[1] = mem[old r1 + (-A)]
>>> 2> stw [old r1], mem[old r1 + (-A)]
>>>
>>> Please notice the stack based on new r1 may be covered with mem[old r1
>>> +(-A)] when addr[old r1 + (-A)] < addr[old r1 + sizeof(an exception frame0].
>>> So the above 2# operation will overwirte something to break this exception
>>> frame then unexpected kernel problem will be issued.
>>>
>>> So looks we have to implement independed interrupt stack for PPC program
>>> exception when CONFIG_BOOKE is enabled. Here we can use
>>> EXC_LEVEL_EXCEPTION_PROLOG to replace original NORMAL_EXCEPTION_PROLOG
>>> for program exception if CONFIG_BOOKE. Then its always safe for kprobe
>>> with independed exc stack from one pre-allocated and dedicated thread_info.
>>> Actually this is just waht we did for critical/machine check exceptions
>>> on PPC.
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver•com>
>>> ---
>> I'm still very confused why we need a unique stack frame for kprobe/program exceptions on book-e devices.
>
> Its a bug at least for Book-E. And if you'd like to check another topic thread,
> "[BUG?]3.0-rc4+ftrace+kprobe: set kprobe at instruction 'stwu' lead to system
> crash/freeze", you can know this story completely :)
>
> This bug should not be reproduced on PPC64 with the exception prolog/endlog
> dedicated to PPC64. But I have no enough time to check other PPC32 & !BOOKE so
> I'm not sure if we should extend this modification.
>
>> Can you explain this further.
>
> I can show one of those issued examples.
>
> Here we kprobe the entry point of show_interrupts().
>
> (gdb) disassemble show_interrupts
> Dump of assembler code for function show_interrupts:
> 0xc0004ff4 <+0>: stwu r1,-48(r1)
> 0xc0004ff8 <+4>: mflr r0
>
> I add some printk() inside pre_handler() to show pt_regs->gpr[1] and pt_regs->nip.
> ------
> ......
> Planted kprobe at c0004ff4
> pre_handler: p->addr = 0xc0004ff4, nip = 0xc0004ff4, msr = 0x29000
> gpr[1] = de767e50.
> nip = c0004ff4.
>
> When hit this instruction, emulate_step() would emulate this instruction as follows:
> ------
> #1> current pr_regs->gpr[1] = 0xde767e50 - 48 = 0xde767e20;
> #2> stw (previous pr_regs->gpr[1]), @(current pr_regs->gpr[1])
> ==> stw (0xde767e50), 0xde767e20
>
> But after this kprobe process something would be rewrite incorrectly:
> ------
> ......
> post_handler: p->addr = 0xc0004ff4, msr = 0x29000
> gpr[1] = de767e20.
> nip = de767e54.
> ^
> If everything is good nip should equal to (0xc0004ff4 + 0x4). But looks its
> reset with (0xde767e50 + 0x4) via the above #2 operation. So why?
>
> As I understand kprobe use 'trap' to enter the program exception. At now PR = 0
> so the kernel allocate an exception frame as normal.
>
> ---------------- old r1[0xde767e50]
> 1 pt_regs->result
> 2 pt_regs->dsisr
> 3 pt_regs->dar
> 4 pt_regs->trap
> 5 pt_regs->mq
> 6 pt_regs->ccr
> 7 pt_regs->xer
> 8 pt_regs->link
> 9 pt_regs->ctr
> 10 pt_regs->orig_gpr3
> 11 pt_regs->msr
> 12 pt_regs->nip <-- @ 0xde767e50 - 12 x 4 = 0xde767e20
> ......
> ----------------- new r1[0xde767e50 - INT_FRAME_SIZE]
>
> I think you can understand why pt_regs->nip is broken :) So the root cause is an
> exception frame and the kprobed function stack frame are always overlap. And
> then someone member inside an exception frame may be corrupted by that emulated
> stw operation.
>
> So I think we have to use one unique stack frame to avoid this when enable
> CONFIG_KPROBES. Especially for Book-E we can refer easily machine
> check/critical/debug exception implementation to do this like my patch.
>
More questions or suggestions?
Tiejun
> Tiejun
>
>> - k
prev parent reply other threads:[~2011-07-21 9:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-11 11:31 [v3 PATCH 1/1] booke/kprobe: make program exception to use one dedicated exception stack Tiejun Chen
2011-07-11 11:31 ` [v3] booke/kprobe: Fix stack corrupt issue when kprobe 'stwu' Tiejun Chen
2011-07-14 11:56 ` tiejun.chen
2011-07-12 2:35 ` [v3 PATCH 1/1] booke/kprobe: make program exception to use one dedicated exception stack tiejun.chen
2011-07-14 13:27 ` Kumar Gala
2011-07-14 15:53 ` Scott Wood
2011-07-15 5:28 ` tiejun.chen
2011-07-15 18:42 ` Scott Wood
2011-07-16 3:25 ` Chen, Tiejun
2011-07-18 15:56 ` Scott Wood
2011-07-19 10:52 ` tiejun.chen
2011-08-30 5:44 ` Benjamin Herrenschmidt
2011-08-31 9:17 ` tiejun.chen
2011-08-31 21:32 ` Benjamin Herrenschmidt
2011-07-21 9:32 ` tiejun.chen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E27F223.4060602@windriver.com \
--to=tiejun.chen@windriver$(echo .)com \
--cc=benjamin.walsh@windriver$(echo .)com \
--cc=galak@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox