[PATCH v3 3/7] riscv: Fixup kprobes handler couldn't change pc

Palmer Dabbelt palmerdabbelt at google.com
Fri Aug 14 18:36:06 EDT 2020


On Mon, 13 Jul 2020 16:39:18 PDT (-0700), guoren at kernel.org wrote:
> From: Guo Ren <guoren at linux.alibaba.com>
>
> The "Changing Execution Path" section in the Documentation/kprobes.txt
> said:
>
> Since kprobes can probe into a running kernel code, it can change the
> register set, including instruction pointer.
>
> Signed-off-by: Guo Ren <guoren at linux.alibaba.com>
> Cc: Masami Hiramatsu <mhiramat at kernel.org>
> Cc: Palmer Dabbelt <palmerdabbelt at google.com>
> ---
>  arch/riscv/kernel/mcount-dyn.S | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> index 35a6ed7..4b58b54 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -123,6 +123,7 @@ ENDPROC(ftrace_caller)
>  	sd	ra, (PT_SIZE_ON_STACK+8)(sp)
>  	addi	s0, sp, (PT_SIZE_ON_STACK+16)
>
> +	sd ra,  PT_EPC(sp)
>  	sd x1,  PT_RA(sp)
>  	sd x2,  PT_SP(sp)
>  	sd x3,  PT_GP(sp)

So that's definately not going to be EPC any more.  I'm not sure that field is
sanely named, though, as it's really just the PC when it comes to other ptrace
stuff.

> @@ -157,6 +158,7 @@ ENDPROC(ftrace_caller)
>  	.endm
>
>  	.macro RESTORE_ALL
> +	ld ra,  PT_EPC(sp)
>  	ld x1,  PT_RA(sp)

x1 is ra, so loading it twice doesn't seem reasonable.

>  	ld x2,  PT_SP(sp)
>  	ld x3,  PT_GP(sp)
> @@ -190,7 +192,6 @@ ENDPROC(ftrace_caller)
>  	ld x31, PT_T6(sp)
>
>  	ld	s0, (PT_SIZE_ON_STACK)(sp)
> -	ld	ra, (PT_SIZE_ON_STACK+8)(sp)
>  	addi	sp, sp, (PT_SIZE_ON_STACK+16)
>  	.endm

If you're dropping the load you should drop the store above as well.  In
general this seems kind of mixed up, both before and after this patch.



More information about the linux-riscv mailing list