[PATCH 0/2] Fix function_graph tracer for ppc64 BE

[PATCH 0/2] Fix function_graph tracer for ppc64 BE

[Naveen N. Rao]

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

[PATCH 1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols"

[Naveen N. Rao]

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

[PATCH 2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text

[Naveen N. Rao]

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

Re: [1/2] Revert "powerpc64/elfv1: Only dereference function descriptor for non-text symbols"

[Michael Ellerman]

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

Re: [2/2] powerpc/kprobes: Dereference function pointers only if the address does not belong to kernel text

[Michael Ellerman]

On Mon, 2017-10-30 at 15:12:09 UTC, "Naveen N. Rao" wrote:
> 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>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/e6c4dcb308160115287afd87afb63b

cheers