[PATCH v2 2/3] powerpc/kprobes: un-blacklist system_call() from kprobes

[Michael Ellerman]

"Naveen N. Rao" <naveen.n.rao at linux.vnet.ibm.com> writes:

> It is actually safe to probe system_call() in entry_64.S, but only till
> .Lsyscall_exit. To allow this, convert .Lsyscall_exit to a non-local
> symbol __system_call() and blacklist that symbol, rather than
> system_call().

I'm not sure I like this. The reason we made it a local symbol in the
first place is because it made backtraces look odd:

  commit 4c3b21686111e0ac6018469dacbc5549f9915cf8
  Author:     Michael Ellerman <mpe at ellerman.id.au>
  AuthorDate: Fri Dec 5 21:16:59 2014 +1100
  
  powerpc/kernel: Make syscall_exit a local label
  
  Currently when we back trace something that is in a syscall we see
  something like this:
  
  [c000000000000000] [c000000000000000] SyS_read+0x6c/0x110
  [c000000000000000] [c000000000000000] syscall_exit+0x0/0x98
  
  Although it's entirely correct, seeing syscall_exit at the bottom can be
  confusing - we were exiting from a syscall and then called SyS_read() ?
  
  If we instead change syscall_exit to be a local label we get something
  more intuitive:
  
  [c0000001fa46fde0] [c00000000026719c] SyS_read+0x6c/0x110
  [c0000001fa46fe30] [c000000000009264] system_call+0x38/0xd0
  
  ie. we were handling a system call, and it was SyS_read().


I think you know that, although you didn't mention it in the change log,
because you've called the new symbol __system_call. But that is not a
great name either because that's not what it does.

> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 380361c0bb6a..e030ce34dd66 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -176,7 +176,7 @@ system_call:			/* label this so stack traces look sane */
>  	mtctr   r12
>  	bctrl			/* Call handler */
>  
> -.Lsyscall_exit:
> +__system_call:
>  	std	r3,RESULT(r1)
>  	CURRENT_THREAD_INFO(r12, r1)
  
Why can't we kprobe the std and the rotate to current thread info?

Is the real no-probe point just here, prior to the clearing of MSR_RI ?

	ld	r8,_MSR(r1)
#ifdef CONFIG_PPC_BOOK3S
	/* No MSR:RI on BookE */

cheers