* [PATCH 0/2] Fix function_graph tracer for ppc64 BE
@ 2017-10-30 15:12 Naveen N. Rao
2017-10-30 15:12 ` [PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols" Naveen N. Rao
2017-10-30 15:12 ` [PATCH 2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text Naveen N. Rao
0 siblings, 2 replies; 5+ messages in thread
From: Naveen N. Rao @ 2017-10-30 15:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Nicholas Piggin, Chandan Rajendra, linuxppc-dev
Chandan reported that trying to enable function_graph tracer on ppc64 BE
now locks up the system. This is due to prepare_ftrace_return() using
ppc_function_entry() for resolving return_to_handler(), which in turn
invokes kernel_text_address(), which also gets traced resulting in a
loop.
We added a check for kernel_text_address() in ppc_function_entry() to
guard all users in case we were called with a function, rather than a
function descriptor. In hindsight, I feel that this is inefficient since
we usually only pass function descriptors to ppc_function_entry() (and
ppc_global_function_entry()). So, I am proposing that we revert the
previous patch and instead implement the necessary checks in the kprobes
subsystem.
The other way to fix this is to simply guard the call to
kernel_text_address() within [un]pause_graph_tracing(), if you think
it's useful to have the check in ppc_function_entry() for all users.
- Naveen
Naveen N. Rao (2):
Revert "powerpc64/elfv1: Only dereference function descriptor for
non-text symbols"
powerpc/kprobes: Dereference function pointers only if the address
does not belong to kernel text
arch/powerpc/include/asm/code-patching.h | 10 +---------
arch/powerpc/kernel/kprobes.c | 7 ++++++-
2 files changed, 7 insertions(+), 10 deletions(-)
--
2.14.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols"
2017-10-30 15:12 [PATCH 0/2] Fix function_graph tracer for ppc64 BE Naveen N. Rao
@ 2017-10-30 15:12 ` Naveen N. Rao
2017-11-02 12:12 ` [1/2] " Michael Ellerman
2017-10-30 15:12 ` [PATCH 2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text Naveen N. Rao
1 sibling, 1 reply; 5+ messages in thread
From: Naveen N. Rao @ 2017-10-30 15:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Nicholas Piggin, Chandan Rajendra, linuxppc-dev
This reverts commit 83e840c770f2c5 ("powerpc64/elfv1: Only dereference
function descriptor for non-text symbols").
Chandan reported that on newer kernels, trying to enable function_graph
tracer on ppc64 (BE) locks up the system with the following trace:
Unable to handle kernel paging request for data at address 0x600000002fa30010
Faulting instruction address: 0xc0000000001f1300
Thread overran stack, or stack corrupted
Oops: Kernel access of bad area, sig: 11 [#1]
BE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
Modules linked in:
CPU: 1 PID: 6586 Comm: bash Not tainted 4.14.0-rc3-00162-g6e51f1f-dirty #20
task: c000000625c07200 task.stack: c000000625c07310
NIP: c0000000001f1300 LR: c000000000121cac CTR: c000000000061af8
REGS: c000000625c088c0 TRAP: 0380 Not tainted (4.14.0-rc3-00162-g6e51f1f-dirty)
MSR: 8000000000001032 <SF,ME,IR,DR,RI> CR: 28002848 XER: 00000000
CFAR: c0000000001f1320 SOFTE: 0
GPR00: c000000000121cac c000000625c08b40 c000000001439700 c00000000125c5a8
GPR04: c0000000013a0040 c00000000135cbf0 0000000000000000 0000000000000000
GPR08: e92d0250812a1a30 e92d025081291a30 600000002fa30000 c000000625c08c50
GPR12: 0000000028002842 c00000000fd40580 000000010bacae64 ffffffffffffffff
GPR16: 000000010bac96c8 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 000000010bac0734 000000000000000a
GPR24: 0000000000000000 c000000636cd6610 c000000113258b80 0000000000000000
GPR28: c000000000061b10 c000000000061960 c00000000125c5d8 c0000000013a0040
NIP [c0000000001f1300] .__is_insn_slot_addr+0x30/0x90
LR [c000000000121cac] .kernel_text_address+0x18c/0x1c0
Call Trace:
[c000000625c08b40] [c0000000001bd040] .is_module_text_address+0x20/0x40 (unreliable)
[c000000625c08bc0] [c000000000121cac] .kernel_text_address+0x18c/0x1c0
[c000000625c08c50] [c000000000061960] .prepare_ftrace_return+0x50/0x130
[c000000625c08cf0] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
[c000000625c08d60] [c000000000121b40] .kernel_text_address+0x20/0x1c0
[c000000625c08df0] [c000000000061960] .prepare_ftrace_return+0x50/0x130
...
[c000000625c0ab30] [c000000000061960] .prepare_ftrace_return+0x50/0x130
[c000000625c0abd0] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
[c000000625c0ac40] [c000000000121b40] .kernel_text_address+0x20/0x1c0
[c000000625c0acd0] [c000000000061960] .prepare_ftrace_return+0x50/0x130
[c000000625c0ad70] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
[c000000625c0ade0] [c000000000121b40] .kernel_text_address+0x20/0x1c0
Instruction dump:
7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78 7c9f2378 60000000
60000000 e95e0031 7faaf040 419e0028 <e92a0010> 7fa9f840 3d090001 7ebf4040
---[ end trace 0870d7d56d703ff4 ]---
This is because ftrace is using ppc_function_entry() for obtaining the
address of return_to_handler() in prepare_ftrace_return(). The call to
kernel_text_address() itself gets traced and we end up in a recursive
loop.
Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/code-patching.h | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 5482928eea1b..abef812de7f8 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -83,16 +83,8 @@ static inline unsigned long ppc_function_entry(void *func)
* On PPC64 ABIv1 the function pointer actually points to the
* function's descriptor. The first entry in the descriptor is the
* address of the function text.
- *
- * However, we may also receive pointer to an assembly symbol. To
- * detect that, we first check if the function pointer we receive
- * already points to kernel/module text and we only dereference it
- * if it doesn't.
*/
- if (kernel_text_address((unsigned long)func))
- return (unsigned long)func;
- else
- return ((func_descr_t *)func)->entry;
+ return ((func_descr_t *)func)->entry;
#else
return (unsigned long)func;
#endif
--
2.14.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols"
2017-10-30 15:12 ` [PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols" Naveen N. Rao
@ 2017-11-02 12:12 ` Michael Ellerman
0 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2017-11-02 12:12 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Chandan Rajendra, linuxppc-dev, Nicholas Piggin
On Mon, 2017-10-30 at 15:12:08 UTC, "Naveen N. Rao" wrote:
> This reverts commit 83e840c770f2c5 ("powerpc64/elfv1: Only dereference
> function descriptor for non-text symbols").
>
> Chandan reported that on newer kernels, trying to enable function_graph
> tracer on ppc64 (BE) locks up the system with the following trace:
>
> Unable to handle kernel paging request for data at address 0x600000002fa30010
> Faulting instruction address: 0xc0000000001f1300
> Thread overran stack, or stack corrupted
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
> Modules linked in:
> CPU: 1 PID: 6586 Comm: bash Not tainted 4.14.0-rc3-00162-g6e51f1f-dirty #20
> task: c000000625c07200 task.stack: c000000625c07310
> NIP: c0000000001f1300 LR: c000000000121cac CTR: c000000000061af8
> REGS: c000000625c088c0 TRAP: 0380 Not tainted (4.14.0-rc3-00162-g6e51f1f-dirty)
> MSR: 8000000000001032 <SF,ME,IR,DR,RI> CR: 28002848 XER: 00000000
> CFAR: c0000000001f1320 SOFTE: 0
> GPR00: c000000000121cac c000000625c08b40 c000000001439700 c00000000125c5a8
> GPR04: c0000000013a0040 c00000000135cbf0 0000000000000000 0000000000000000
> GPR08: e92d0250812a1a30 e92d025081291a30 600000002fa30000 c000000625c08c50
> GPR12: 0000000028002842 c00000000fd40580 000000010bacae64 ffffffffffffffff
> GPR16: 000000010bac96c8 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 000000010bac0734 000000000000000a
> GPR24: 0000000000000000 c000000636cd6610 c000000113258b80 0000000000000000
> GPR28: c000000000061b10 c000000000061960 c00000000125c5d8 c0000000013a0040
> NIP [c0000000001f1300] .__is_insn_slot_addr+0x30/0x90
> LR [c000000000121cac] .kernel_text_address+0x18c/0x1c0
> Call Trace:
> [c000000625c08b40] [c0000000001bd040] .is_module_text_address+0x20/0x40 (unreliable)
> [c000000625c08bc0] [c000000000121cac] .kernel_text_address+0x18c/0x1c0
> [c000000625c08c50] [c000000000061960] .prepare_ftrace_return+0x50/0x130
> [c000000625c08cf0] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
> [c000000625c08d60] [c000000000121b40] .kernel_text_address+0x20/0x1c0
> [c000000625c08df0] [c000000000061960] .prepare_ftrace_return+0x50/0x130
> ...
> [c000000625c0ab30] [c000000000061960] .prepare_ftrace_return+0x50/0x130
> [c000000625c0abd0] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
> [c000000625c0ac40] [c000000000121b40] .kernel_text_address+0x20/0x1c0
> [c000000625c0acd0] [c000000000061960] .prepare_ftrace_return+0x50/0x130
> [c000000625c0ad70] [c000000000061b10] .ftrace_graph_caller+0x14/0x34
> [c000000625c0ade0] [c000000000121b40] .kernel_text_address+0x20/0x1c0
> Instruction dump:
> 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 7c7e1b78 7c9f2378 60000000
> 60000000 e95e0031 7faaf040 419e0028 <e92a0010> 7fa9f840 3d090001 7ebf4040
> ---[ end trace 0870d7d56d703ff4 ]---
>
> This is because ftrace is using ppc_function_entry() for obtaining the
> address of return_to_handler() in prepare_ftrace_return(). The call to
> kernel_text_address() itself gets traced and we end up in a recursive
> loop.
>
> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/63be1a81e40733ecd175713b6a7558
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text
2017-10-30 15:12 [PATCH 0/2] Fix function_graph tracer for ppc64 BE Naveen N. Rao
2017-10-30 15:12 ` [PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols" Naveen N. Rao
@ 2017-10-30 15:12 ` Naveen N. Rao
2017-11-02 12:12 ` [2/2] " Michael Ellerman
1 sibling, 1 reply; 5+ messages in thread
From: Naveen N. Rao @ 2017-10-30 15:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Nicholas Piggin, Chandan Rajendra, linuxppc-dev
This makes the changes introduced in commit 83e840c770f2c5
("powerpc64/elfv1: Only dereference function descriptor for non-text
symbols") to be specific to the kprobe subsystem.
We previously changed ppc_function_entry() to always check the provided
address to confirm if it needed to be dereferenced. This is actually
only an issue for kprobe blacklisted asm labels (through use of
_ASM_NOKPROBE_SYMBOL) and can cause other issues with ftrace. Also, the
additional checks are not really necessary for our other uses.
As such, move this check to the kprobes subsystem.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/kprobes.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index bbb4795660f8..ca5d5a081e75 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -598,7 +598,12 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);
unsigned long arch_deref_entry_point(void *entry)
{
- return ppc_global_function_entry(entry);
+#ifdef PPC64_ELF_ABI_v1
+ if (!kernel_text_address((unsigned long)entry))
+ return ppc_global_function_entry(entry);
+ else
+#endif
+ return (unsigned long)entry;
}
NOKPROBE_SYMBOL(arch_deref_entry_point);
--
2.14.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-02 12:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-30 15:12 [PATCH 0/2] Fix function_graph tracer for ppc64 BE Naveen N. Rao
2017-10-30 15:12 ` [PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols" Naveen N. Rao
2017-11-02 12:12 ` [1/2] " Michael Ellerman
2017-10-30 15:12 ` [PATCH 2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text Naveen N. Rao
2017-11-02 12:12 ` [2/2] " Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).