[PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds

Tue Feb 1 05:18:39 PST 2022

On Mon, 31 Jan 2022 18:03:44 +0100
Ard Biesheuvel <ardb at kernel.org> wrote:

> Thumb2 code uses R7 as the frame pointer rather than R11, because the
> opcodes to access it are generally shorter.
> 
> This means that there are cases where we cannot simply add it to the
> clobber list of an asm() block, but need to preserve/restore it
> explicitly, or the compiler may complain in some cases (e.g., Clang
> builds with ftrace enabled).
> 
> Since R11 is not special in that case, clobber it instead, and use it to
> preserve/restore the value of R7.

Thanks Ard for fixing thumb2 issue!
BTW, have you build the kernel with CONFIG_KPROBES_SANITY_TEST=y?
It should check the backtrace from kprobe and kretprobe at boot time.

Thank you,

> 
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm/include/asm/opcodes.h           | 14 ++++++++
>  arch/arm/probes/kprobes/actions-common.c |  6 ++--
>  arch/arm/probes/kprobes/actions-thumb.c  | 38 ++++++++++++++++----
>  3 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h
> index 38e3eabff5c3..9a6362408ea0 100644
> --- a/arch/arm/include/asm/opcodes.h
> +++ b/arch/arm/include/asm/opcodes.h
> @@ -230,4 +230,18 @@ extern __u32 __opcode_to_mem_thumb32(__u32);
>  	".short " __stringify(first) ", " __stringify(second) "\n\t"
>  #endif
>  
> +/*
> + * Which register to preserve and which register can be clobbered in inline asm
> + * that needs to be compatible with code that emits frame pointers.
> + */
> +#ifdef CONFIG_THUMB2_KERNEL
> +#define FPREG_PRESERVE	"r7"
> +#define FPREG_CLOBBER	"r11"
> +#define FPREG_PRESERVE_R7
> +#else
> +#define FPREG_PRESERVE	"fp"
> +#define FPREG_CLOBBER	"r7"
> +#define FPREG_PRESERVE_R11
> +#endif
> +
>  #endif /* __ASM_ARM_OPCODES_H */
> diff --git a/arch/arm/probes/kprobes/actions-common.c b/arch/arm/probes/kprobes/actions-common.c
> index 836aebe596cd..f0efe16e2fdb 100644
> --- a/arch/arm/probes/kprobes/actions-common.c
> +++ b/arch/arm/probes/kprobes/actions-common.c
> @@ -84,7 +84,7 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,
>  	register void *rfn asm("lr") = asi->insn_fn;
>  
>  	__asm__ __volatile__ (
> -		"stmdb	sp!, {%[regs], r11}	\n\t"
> +		"stmdb	sp!, {%[regs], " FPREG_PRESERVE "}\n\t"
>  		"ldmia	%[regs], {r0-r12}	\n\t"
>  #if __LINUX_ARM_ARCH__ >= 6
>  		"blx	%[fn]			\n\t"
> @@ -96,10 +96,10 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,
>  #endif
>  		"ldr	lr, [sp], #4		\n\t" /* lr = regs */
>  		"stmia	lr, {r0-r12}		\n\t"
> -		"ldr	r11, [sp], #4		\n\t"
> +		"ldr	" FPREG_PRESERVE ", [sp], #4\n\t"
>  		: [regs] "=r" (rregs), [fn] "=r" (rfn)
>  		: "0" (rregs), "1" (rfn)
> -		: "r0", "r2", "r3", "r4", "r5", "r6", "r7",
> +		: "r0", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,
>  		  "r8", "r9", "r10", "r12", "memory", "cc"
>  		);
>  }
> diff --git a/arch/arm/probes/kprobes/actions-thumb.c b/arch/arm/probes/kprobes/actions-thumb.c
> index 7884fcb81c26..873757326533 100644
> --- a/arch/arm/probes/kprobes/actions-thumb.c
> +++ b/arch/arm/probes/kprobes/actions-thumb.c
> @@ -447,14 +447,20 @@ t16_emulate_loregs(probes_opcode_t insn,
>  
>  	__asm__ __volatile__ (
>  		"msr	cpsr_fs, %[oldcpsr]	\n\t"
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	fp, r7			\n\t"
> +#endif
>  		"ldmia	%[regs], {r0-r7}	\n\t"
>  		"blx	%[fn]			\n\t"
>  		"stmia	%[regs], {r0-r7}	\n\t"
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	r7, fp			\n\t"
> +#endif
>  		"mrs	%[newcpsr], cpsr	\n\t"
>  		: [newcpsr] "=r" (newcpsr)
>  		: [oldcpsr] "r" (oldcpsr), [regs] "r" (regs),
>  		  [fn] "r" (asi->insn_fn)
> -		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> +		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,
>  		  "lr", "memory", "cc"
>  		);
>  
> @@ -524,15 +530,21 @@ t16_emulate_push(probes_opcode_t insn,
>  		struct arch_probes_insn *asi, struct pt_regs *regs)
>  {
>  	__asm__ __volatile__ (
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	fp, r7			\n\t"
> +#endif
>  		"ldr	r9, [%[regs], #13*4]	\n\t"
>  		"ldr	r8, [%[regs], #14*4]	\n\t"
>  		"ldmia	%[regs], {r0-r7}	\n\t"
>  		"blx	%[fn]			\n\t"
>  		"str	r9, [%[regs], #13*4]	\n\t"
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	r7, fp			\n\t"
> +#endif
>  		:
>  		: [regs] "r" (regs), [fn] "r" (asi->insn_fn)
> -		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8", "r9",
> -		  "lr", "memory", "cc"
> +		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r8", "r9",
> +		  FPREG_CLOBBER, "lr", "memory", "cc"
>  		);
>  }
>  
> @@ -558,15 +570,21 @@ t16_emulate_pop_nopc(probes_opcode_t insn,
>  		struct arch_probes_insn *asi, struct pt_regs *regs)
>  {
>  	__asm__ __volatile__ (
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	fp, r7			\n\t"
> +#endif
>  		"ldr	r9, [%[regs], #13*4]	\n\t"
>  		"ldmia	%[regs], {r0-r7}	\n\t"
>  		"blx	%[fn]			\n\t"
>  		"stmia	%[regs], {r0-r7}	\n\t"
>  		"str	r9, [%[regs], #13*4]	\n\t"
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	r7, fp			\n\t"
> +#endif
>  		:
>  		: [regs] "r" (regs), [fn] "r" (asi->insn_fn)
> -		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r9",
> -		  "lr", "memory", "cc"
> +		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r9",
> +		  FPREG_CLOBBER, "lr", "memory", "cc"
>  		);
>  }
>  
> @@ -577,15 +595,21 @@ t16_emulate_pop_pc(probes_opcode_t insn,
>  	register unsigned long pc asm("r8");
>  
>  	__asm__ __volatile__ (
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	fp, r7			\n\t"
> +#endif
>  		"ldr	r9, [%[regs], #13*4]	\n\t"
>  		"ldmia	%[regs], {r0-r7}	\n\t"
>  		"blx	%[fn]			\n\t"
>  		"stmia	%[regs], {r0-r7}	\n\t"
>  		"str	r9, [%[regs], #13*4]	\n\t"
> +#ifdef FPREG_PRESERVE_R7
> +		"mov	r7, fp			\n\t"
> +#endif
>  		: "=r" (pc)
>  		: [regs] "r" (regs), [fn] "r" (asi->insn_fn)
> -		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r9",
> -		  "lr", "memory", "cc"
> +		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r9",
> +		  FPREG_CLOBBER, "lr", "memory", "cc"
>  		);
>  
>  	bx_write_pc(pc, regs);
> -- 
> 2.30.2
> 

-- 
Masami Hiramatsu <mhiramat at kernel.org>