| Message ID | 1443581395-30088-1-git-send-email-huawei.libin@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/30/2015 11:49 AM, Li Bin wrote: > When function graph tracer is enabled, the following operation > will trigger panic: > > mount -t debugfs nodev /sys/kernel > echo next_tgid > /sys/kernel/tracing/set_ftrace_filter > echo function_graph > /sys/kernel/tracing/current_tracer > ls /proc/ > > ------------[ cut here ]------------ > [ 198.501417] Unable to handle kernel paging request at virtual address cb88537fdc8ba316 > [ 198.506126] pgd = ffffffc008f79000 > [ 198.509363] [cb88537fdc8ba316] *pgd=00000000488c6003, *pud=00000000488c6003, *pmd=0000000000000000 > [ 198.517726] Internal error: Oops: 94000005 [#1] SMP > [ 198.518798] Modules linked in: > [ 198.520582] CPU: 1 PID: 1388 Comm: ls Tainted: G > [ 198.521800] Hardware name: linux,dummy-virt (DT) > [ 198.522852] task: ffffffc0fa9e8000 ti: ffffffc0f9ab0000 task.ti: ffffffc0f9ab0000 > [ 198.524306] PC is at next_tgid+0x30/0x100 > [ 198.525205] LR is at return_to_handler+0x0/0x20 > [ 198.526090] pc : [<ffffffc0002a1070>] lr : [<ffffffc0000907c0>] pstate: 60000145 > [ 198.527392] sp : ffffffc0f9ab3d40 > [ 198.528084] x29: ffffffc0f9ab3d40 x28: ffffffc0f9ab0000 > [ 198.529406] x27: ffffffc000d6a000 x26: ffffffc000b786e8 > [ 198.530659] x25: ffffffc0002a1900 x24: ffffffc0faf16c00 > [ 198.531942] x23: ffffffc0f9ab3ea0 x22: 0000000000000002 > [ 198.533202] x21: ffffffc000d85050 x20: 0000000000000002 > [ 198.534446] x19: 0000000000000002 x18: 0000000000000000 > [ 198.535719] x17: 000000000049fa08 x16: ffffffc000242efc > [ 198.537030] x15: 0000007fa472b54c x14: ffffffffff000000 > [ 198.538347] x13: ffffffc0fada84a0 x12: 0000000000000001 > [ 198.539634] x11: ffffffc0f9ab3d70 x10: ffffffc0f9ab3d70 > [ 198.540915] x9 : ffffffc0000907c0 x8 : ffffffc0f9ab3d40 > [ 198.542215] x7 : 0000002e330f08f0 x6 : 0000000000000015 > [ 198.543508] x5 : 0000000000000f08 x4 : ffffffc0f9835ec0 > [ 198.544792] x3 : cb88537fdc8ba316 x2 : cb88537fdc8ba306 > [ 198.546108] x1 : 0000000000000002 x0 : ffffffc000d85050 > [ 198.547432] > [ 198.547920] Process ls (pid: 1388, stack limit = 0xffffffc0f9ab0020) > [ 198.549170] Stack: (0xffffffc0f9ab3d40 to 0xffffffc0f9ab4000) > [ 198.582568] Call trace: > [ 198.583313] [<ffffffc0002a1070>] next_tgid+0x30/0x100 > [ 198.584359] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > [ 198.585503] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > [ 198.586574] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > [ 198.587660] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > [ 198.588896] Code: aa0003f5 2a0103f4 b4000102 91004043 (885f7c60) > [ 198.591092] ---[ end trace 6a346f8f20949ac8 ]--- > > This is because when using function graph tracer, if the traced > function return value is in multi regs ([0x-07]), return_to_handler > may corrupt them. So in return_to_handler, the parameter regs should > be protected properly. You're right. we should preserve x0-x7 around a call to ftrace_return_to_handler() just in case they might be used as a "composite type" (ie. struct) of return value. Thanks, -Takahiro AKASHI > Cc: <stable@vger.kernel.org> # 3.18+ > Signed-off-by: Li Bin <huawei.libin@huawei.com> > --- > arch/arm64/kernel/entry-ftrace.S | 22 ++++++++++++++++++++-- > 1 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index 08cafc5..0f03a8f 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -178,6 +178,24 @@ ENTRY(ftrace_stub) > ENDPROC(ftrace_stub) > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > + /* save return value regs*/ > + .macro save_return_regs > + sub sp, sp, #64 > + stp x0, x1, [sp] > + stp x2, x3, [sp, #16] > + stp x4, x5, [sp, #32] > + stp x6, x7, [sp, #48] > + .endm > + > + /* restore return value regs*/ > + .macro restore_return_regs > + ldp x0, x1, [sp] > + ldp x2, x3, [sp, #16] > + ldp x4, x5, [sp, #32] > + ldp x6, x7, [sp, #48] > + add sp, sp, #64 > + .endm > + > /* > * void ftrace_graph_caller(void) > * > @@ -204,11 +222,11 @@ ENDPROC(ftrace_graph_caller) > * only when CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST is enabled. > */ > ENTRY(return_to_handler) > - str x0, [sp, #-16]! > + save_return_regs > mov x0, x29 // parent's fp > bl ftrace_return_to_handler// addr = ftrace_return_to_hander(fp); > mov x30, x0 // restore the original return address > - ldr x0, [sp], #16 > + restore_return_regs > ret > END(return_to_handler) > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ >
On Thu, Oct 01, 2015 at 03:11:29PM +0900, AKASHI Takahiro wrote: > On 09/30/2015 11:49 AM, Li Bin wrote: > >When function graph tracer is enabled, the following operation > >will trigger panic: > > > >mount -t debugfs nodev /sys/kernel > >echo next_tgid > /sys/kernel/tracing/set_ftrace_filter > >echo function_graph > /sys/kernel/tracing/current_tracer > >ls /proc/ > > > >------------[ cut here ]------------ > >[ 198.501417] Unable to handle kernel paging request at virtual address cb88537fdc8ba316 > >[ 198.506126] pgd = ffffffc008f79000 > >[ 198.509363] [cb88537fdc8ba316] *pgd=00000000488c6003, *pud=00000000488c6003, *pmd=0000000000000000 > >[ 198.517726] Internal error: Oops: 94000005 [#1] SMP > >[ 198.518798] Modules linked in: > >[ 198.520582] CPU: 1 PID: 1388 Comm: ls Tainted: G > >[ 198.521800] Hardware name: linux,dummy-virt (DT) > >[ 198.522852] task: ffffffc0fa9e8000 ti: ffffffc0f9ab0000 task.ti: ffffffc0f9ab0000 > >[ 198.524306] PC is at next_tgid+0x30/0x100 > >[ 198.525205] LR is at return_to_handler+0x0/0x20 > >[ 198.526090] pc : [<ffffffc0002a1070>] lr : [<ffffffc0000907c0>] pstate: 60000145 > >[ 198.527392] sp : ffffffc0f9ab3d40 > >[ 198.528084] x29: ffffffc0f9ab3d40 x28: ffffffc0f9ab0000 > >[ 198.529406] x27: ffffffc000d6a000 x26: ffffffc000b786e8 > >[ 198.530659] x25: ffffffc0002a1900 x24: ffffffc0faf16c00 > >[ 198.531942] x23: ffffffc0f9ab3ea0 x22: 0000000000000002 > >[ 198.533202] x21: ffffffc000d85050 x20: 0000000000000002 > >[ 198.534446] x19: 0000000000000002 x18: 0000000000000000 > >[ 198.535719] x17: 000000000049fa08 x16: ffffffc000242efc > >[ 198.537030] x15: 0000007fa472b54c x14: ffffffffff000000 > >[ 198.538347] x13: ffffffc0fada84a0 x12: 0000000000000001 > >[ 198.539634] x11: ffffffc0f9ab3d70 x10: ffffffc0f9ab3d70 > >[ 198.540915] x9 : ffffffc0000907c0 x8 : ffffffc0f9ab3d40 > >[ 198.542215] x7 : 0000002e330f08f0 x6 : 0000000000000015 > >[ 198.543508] x5 : 0000000000000f08 x4 : ffffffc0f9835ec0 > >[ 198.544792] x3 : cb88537fdc8ba316 x2 : cb88537fdc8ba306 > >[ 198.546108] x1 : 0000000000000002 x0 : ffffffc000d85050 > >[ 198.547432] > >[ 198.547920] Process ls (pid: 1388, stack limit = 0xffffffc0f9ab0020) > >[ 198.549170] Stack: (0xffffffc0f9ab3d40 to 0xffffffc0f9ab4000) > >[ 198.582568] Call trace: > >[ 198.583313] [<ffffffc0002a1070>] next_tgid+0x30/0x100 > >[ 198.584359] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > >[ 198.585503] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > >[ 198.586574] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > >[ 198.587660] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 > >[ 198.588896] Code: aa0003f5 2a0103f4 b4000102 91004043 (885f7c60) > >[ 198.591092] ---[ end trace 6a346f8f20949ac8 ]--- > > > >This is because when using function graph tracer, if the traced > >function return value is in multi regs ([0x-07]), return_to_handler > >may corrupt them. So in return_to_handler, the parameter regs should > >be protected properly. > > You're right. we should preserve x0-x7 around a call to ftrace_return_to_handler() > just in case they might be used as a "composite type" (ie. struct) of return value. Do I take this as an ack? I applied the patch locally and I'm going to send a pull request tomorrow. Thanks.
> Do I take this as an ack? Yes, but On 10/02/2015 12:27 AM, Catalin Marinas wrote: > On Thu, Oct 01, 2015 at 03:11:29PM +0900, AKASHI Takahiro wrote: >> On 09/30/2015 11:49 AM, Li Bin wrote: >>> When function graph tracer is enabled, the following operation >>> will trigger panic: >>> >>> mount -t debugfs nodev /sys/kernel >>> echo next_tgid > /sys/kernel/tracing/set_ftrace_filter >>> echo function_graph > /sys/kernel/tracing/current_tracer >>> ls /proc/ >>> >>> ------------[ cut here ]------------ >>> [ 198.501417] Unable to handle kernel paging request at virtual address cb88537fdc8ba316 >>> [ 198.506126] pgd = ffffffc008f79000 >>> [ 198.509363] [cb88537fdc8ba316] *pgd=00000000488c6003, *pud=00000000488c6003, *pmd=0000000000000000 >>> [ 198.517726] Internal error: Oops: 94000005 [#1] SMP >>> [ 198.518798] Modules linked in: >>> [ 198.520582] CPU: 1 PID: 1388 Comm: ls Tainted: G >>> [ 198.521800] Hardware name: linux,dummy-virt (DT) >>> [ 198.522852] task: ffffffc0fa9e8000 ti: ffffffc0f9ab0000 task.ti: ffffffc0f9ab0000 >>> [ 198.524306] PC is at next_tgid+0x30/0x100 >>> [ 198.525205] LR is at return_to_handler+0x0/0x20 >>> [ 198.526090] pc : [<ffffffc0002a1070>] lr : [<ffffffc0000907c0>] pstate: 60000145 >>> [ 198.527392] sp : ffffffc0f9ab3d40 >>> [ 198.528084] x29: ffffffc0f9ab3d40 x28: ffffffc0f9ab0000 >>> [ 198.529406] x27: ffffffc000d6a000 x26: ffffffc000b786e8 >>> [ 198.530659] x25: ffffffc0002a1900 x24: ffffffc0faf16c00 >>> [ 198.531942] x23: ffffffc0f9ab3ea0 x22: 0000000000000002 >>> [ 198.533202] x21: ffffffc000d85050 x20: 0000000000000002 >>> [ 198.534446] x19: 0000000000000002 x18: 0000000000000000 >>> [ 198.535719] x17: 000000000049fa08 x16: ffffffc000242efc >>> [ 198.537030] x15: 0000007fa472b54c x14: ffffffffff000000 >>> [ 198.538347] x13: ffffffc0fada84a0 x12: 0000000000000001 >>> [ 198.539634] x11: ffffffc0f9ab3d70 x10: ffffffc0f9ab3d70 >>> [ 198.540915] x9 : ffffffc0000907c0 x8 : ffffffc0f9ab3d40 >>> [ 198.542215] x7 : 0000002e330f08f0 x6 : 0000000000000015 >>> [ 198.543508] x5 : 0000000000000f08 x4 : ffffffc0f9835ec0 >>> [ 198.544792] x3 : cb88537fdc8ba316 x2 : cb88537fdc8ba306 >>> [ 198.546108] x1 : 0000000000000002 x0 : ffffffc000d85050 >>> [ 198.547432] >>> [ 198.547920] Process ls (pid: 1388, stack limit = 0xffffffc0f9ab0020) >>> [ 198.549170] Stack: (0xffffffc0f9ab3d40 to 0xffffffc0f9ab4000) >>> [ 198.582568] Call trace: >>> [ 198.583313] [<ffffffc0002a1070>] next_tgid+0x30/0x100 >>> [ 198.584359] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 >>> [ 198.585503] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 >>> [ 198.586574] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 >>> [ 198.587660] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 >>> [ 198.588896] Code: aa0003f5 2a0103f4 b4000102 91004043 (885f7c60) >>> [ 198.591092] ---[ end trace 6a346f8f20949ac8 ]--- >>> >>> This is because when using function graph tracer, if the traced >>> function return value is in multi regs ([0x-07]), return_to_handler typo: 0x-07 => x0-x7 and pre/post-indexed addressing stp&ldp may save add&sub instructions, but it's a matter of preference. -Takahiro AKASHI >>> may corrupt them. So in return_to_handler, the parameter regs should >>> be protected properly. >> >> You're right. we should preserve x0-x7 around a call to ftrace_return_to_handler() >> just in case they might be used as a "composite type" (ie. struct) of return value. > > Do I take this as an ack? > > I applied the patch locally and I'm going to send a pull request > tomorrow. > > Thanks. >
On Fri, Oct 02, 2015 at 04:56:48PM +0900, AKASHI Takahiro wrote: > >>On 09/30/2015 11:49 AM, Li Bin wrote: > >>>This is because when using function graph tracer, if the traced > >>>function return value is in multi regs ([0x-07]), return_to_handler > > typo: 0x-07 => x0-x7 I fixed this up. > and pre/post-indexed addressing stp&ldp may save add&sub instructions, but > it's a matter of preference. Doing it this way is more efficient in general as it avoids updating the sp and writing in reverse order. That's the reason we recently changed the kernel_entry/exit macros (see commit 63648dd20fa0 "arm64: entry: use ldp/stp instead of push/pop when saving/restoring regs"). Thanks.
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S index 08cafc5..0f03a8f 100644 --- a/arch/arm64/kernel/entry-ftrace.S +++ b/arch/arm64/kernel/entry-ftrace.S @@ -178,6 +178,24 @@ ENTRY(ftrace_stub) ENDPROC(ftrace_stub) #ifdef CONFIG_FUNCTION_GRAPH_TRACER + /* save return value regs*/ + .macro save_return_regs + sub sp, sp, #64 + stp x0, x1, [sp] + stp x2, x3, [sp, #16] + stp x4, x5, [sp, #32] + stp x6, x7, [sp, #48] + .endm + + /* restore return value regs*/ + .macro restore_return_regs + ldp x0, x1, [sp] + ldp x2, x3, [sp, #16] + ldp x4, x5, [sp, #32] + ldp x6, x7, [sp, #48] + add sp, sp, #64 + .endm + /* * void ftrace_graph_caller(void) * @@ -204,11 +222,11 @@ ENDPROC(ftrace_graph_caller) * only when CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST is enabled. */ ENTRY(return_to_handler) - str x0, [sp, #-16]! + save_return_regs mov x0, x29 // parent's fp bl ftrace_return_to_handler// addr = ftrace_return_to_hander(fp); mov x30, x0 // restore the original return address - ldr x0, [sp], #16 + restore_return_regs ret END(return_to_handler) #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
When function graph tracer is enabled, the following operation will trigger panic: mount -t debugfs nodev /sys/kernel echo next_tgid > /sys/kernel/tracing/set_ftrace_filter echo function_graph > /sys/kernel/tracing/current_tracer ls /proc/ ------------[ cut here ]------------ [ 198.501417] Unable to handle kernel paging request at virtual address cb88537fdc8ba316 [ 198.506126] pgd = ffffffc008f79000 [ 198.509363] [cb88537fdc8ba316] *pgd=00000000488c6003, *pud=00000000488c6003, *pmd=0000000000000000 [ 198.517726] Internal error: Oops: 94000005 [#1] SMP [ 198.518798] Modules linked in: [ 198.520582] CPU: 1 PID: 1388 Comm: ls Tainted: G [ 198.521800] Hardware name: linux,dummy-virt (DT) [ 198.522852] task: ffffffc0fa9e8000 ti: ffffffc0f9ab0000 task.ti: ffffffc0f9ab0000 [ 198.524306] PC is at next_tgid+0x30/0x100 [ 198.525205] LR is at return_to_handler+0x0/0x20 [ 198.526090] pc : [<ffffffc0002a1070>] lr : [<ffffffc0000907c0>] pstate: 60000145 [ 198.527392] sp : ffffffc0f9ab3d40 [ 198.528084] x29: ffffffc0f9ab3d40 x28: ffffffc0f9ab0000 [ 198.529406] x27: ffffffc000d6a000 x26: ffffffc000b786e8 [ 198.530659] x25: ffffffc0002a1900 x24: ffffffc0faf16c00 [ 198.531942] x23: ffffffc0f9ab3ea0 x22: 0000000000000002 [ 198.533202] x21: ffffffc000d85050 x20: 0000000000000002 [ 198.534446] x19: 0000000000000002 x18: 0000000000000000 [ 198.535719] x17: 000000000049fa08 x16: ffffffc000242efc [ 198.537030] x15: 0000007fa472b54c x14: ffffffffff000000 [ 198.538347] x13: ffffffc0fada84a0 x12: 0000000000000001 [ 198.539634] x11: ffffffc0f9ab3d70 x10: ffffffc0f9ab3d70 [ 198.540915] x9 : ffffffc0000907c0 x8 : ffffffc0f9ab3d40 [ 198.542215] x7 : 0000002e330f08f0 x6 : 0000000000000015 [ 198.543508] x5 : 0000000000000f08 x4 : ffffffc0f9835ec0 [ 198.544792] x3 : cb88537fdc8ba316 x2 : cb88537fdc8ba306 [ 198.546108] x1 : 0000000000000002 x0 : ffffffc000d85050 [ 198.547432] [ 198.547920] Process ls (pid: 1388, stack limit = 0xffffffc0f9ab0020) [ 198.549170] Stack: (0xffffffc0f9ab3d40 to 0xffffffc0f9ab4000) [ 198.582568] Call trace: [ 198.583313] [<ffffffc0002a1070>] next_tgid+0x30/0x100 [ 198.584359] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 [ 198.585503] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 [ 198.586574] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 [ 198.587660] [<ffffffc0000907bc>] ftrace_graph_caller+0x6c/0x70 [ 198.588896] Code: aa0003f5 2a0103f4 b4000102 91004043 (885f7c60) [ 198.591092] ---[ end trace 6a346f8f20949ac8 ]--- This is because when using function graph tracer, if the traced function return value is in multi regs ([0x-07]), return_to_handler may corrupt them. So in return_to_handler, the parameter regs should be protected properly. Cc: <stable@vger.kernel.org> # 3.18+ Signed-off-by: Li Bin <huawei.libin@huawei.com> --- arch/arm64/kernel/entry-ftrace.S | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)