Re: [PATCH 2/2] powerpc/jprobes: Validate break handler invocation as being due to a jprobe_return()

[Masami Hiramatsu <mhiramat@kernel.org>]

On Mon, 15 May 2017 23:35:04 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Fix a circa 2005 FIXME by implementing a check to ensure that we
> actually got into the jprobe break handler() due to the trap in
> jprobe_return().
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/kprobes.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 19b053475758..1ebeb8c482db 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -627,25 +627,23 @@ NOKPROBE_SYMBOL(setjmp_pre_handler);
>  
>  void __used jprobe_return(void)
>  {
> -	asm volatile("trap" ::: "memory");
> +	asm volatile("jprobe_return_trap:\n"
> +		     "trap\n"
> +		     ::: "memory");
>  }
>  NOKPROBE_SYMBOL(jprobe_return);
>  
> -static void __used jprobe_return_end(void)
> -{
> -}
> -NOKPROBE_SYMBOL(jprobe_return_end);
> -
>  int longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
>  {
>  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>  	unsigned long sp;
>  
> -	/*
> -	 * FIXME - we should ideally be validating that we got here 'cos
> -	 * of the "trap" in jprobe_return() above, before restoring the
> -	 * saved regs...
> -	 */
> +	if (regs->nip != ppc_kallsyms_lookup_name("jprobe_return_trap")) {
> +		WARN(1, "longjmp_break_handler NIP (0x%lx) does not match jprobe_return_trap (0x%lx)\n",
> +				regs->nip, ppc_kallsyms_lookup_name("jprobe_return_trap"));
> +		return 0;

If you don't handle this break, you shouldn't warn it, because
program_check_exception() will continue to find how to handle it
by notify_die(). IOW, please return silently, or just add a
debug message.

Thank you,

> +	}
> +
>  	memcpy(regs, &kcb->jprobe_saved_regs, sizeof(struct pt_regs));
>  	sp = kernel_stack_pointer(regs);
>  	memcpy((void *)sp, &kcb->jprobes_stack, MIN_STACK_SIZE(sp));
> -- 
> 2.12.2
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>