Re: [PATCH v11 07/12] ARM: rethook: Add rethook arm implementation

[Masami Hiramatsu <mhiramat@xxxxxxxxxx>]

On Sat, 12 Mar 2022 01:00:03 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:

> +void __naked arch_rethook_trampoline(void)
> +{
> +	__asm__ __volatile__ (
> +#ifdef CONFIG_FRAME_POINTER
> +		"ldr	lr, =arch_rethook_trampoline	\n\t"

Oops, this must have the same issue reported by 0day build bot recently[1].

[1] https://lore.kernel.org/all/202203150516.KTorSVVU-lkp@xxxxxxxxx/T/#u

I'll update this series with same fix.

Thank you,

> +	/* this makes a framepointer on pt_regs. */
> +#ifdef CONFIG_CC_IS_CLANG
> +		"stmdb	sp, {sp, lr, pc}	\n\t"
> +		"sub	sp, sp, #12		\n\t"
> +		/* In clang case, pt_regs->ip = lr. */
> +		"stmdb	sp!, {r0 - r11, lr}	\n\t"
> +		/* fp points regs->r11 (fp) */
> +		"add	fp, sp,	#44		\n\t"
> +#else /* !CONFIG_CC_IS_CLANG */
> +		/* In gcc case, pt_regs->ip = fp. */
> +		"stmdb	sp, {fp, sp, lr, pc}	\n\t"
> +		"sub	sp, sp, #16		\n\t"
> +		"stmdb	sp!, {r0 - r11}		\n\t"
> +		/* fp points regs->r15 (pc) */
> +		"add	fp, sp, #60		\n\t"
> +#endif /* CONFIG_CC_IS_CLANG */
> +#else /* !CONFIG_FRAME_POINTER */
> +		"sub	sp, sp, #16		\n\t"
> +		"stmdb	sp!, {r0 - r11}		\n\t"
> +#endif /* CONFIG_FRAME_POINTER */
> +		"mov	r0, sp			\n\t"
> +		"bl	arch_rethook_trampoline_callback	\n\t"
> +		"mov	lr, r0			\n\t"
> +		"ldmia	sp!, {r0 - r11}		\n\t"
> +		"add	sp, sp, #16		\n\t"
> +#ifdef CONFIG_THUMB2_KERNEL
> +		"bx	lr			\n\t"
> +#else
> +		"mov	pc, lr			\n\t"
> +#endif
> +		: : : "memory");
> +}
> +NOKPROBE_SYMBOL(arch_rethook_trampoline);
> +
> +/*
> + * At the entry of function with mcount. The stack and registers are prepared
> + * for the mcount function as below.
> + *
> + * mov     ip, sp
> + * push    {fp, ip, lr, pc}
> + * sub     fp, ip, #4	; FP[0] = PC, FP[-4] = LR, and FP[-12] = call-site FP.
> + * push    {lr}
> + * bl      <__gnu_mcount_nc> ; call ftrace
> + *
> + * And when returning from the function, call-site FP, SP and PC are restored
> + * from stack as below;
> + *
> + * ldm     sp, {fp, sp, pc}
> + *
> + * Thus, if the arch_rethook_prepare() is called from real function entry,
> + * it must change the LR and save FP in pt_regs. But if it is called via
> + * mcount context (ftrace), it must change the LR on stack, which is next
> + * to the PC (= FP[-4]), and save the FP value at FP[-12].
> + */
> +void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount)
> +{
> +	unsigned long *ret_addr, *frame;
> +
> +	if (mcount) {
> +		ret_addr = (unsigned long *)(regs->ARM_fp - 4);
> +		frame = (unsigned long *)(regs->ARM_fp - 12);
> +	} else {
> +		ret_addr = &regs->ARM_lr;
> +		frame = &regs->ARM_fp;
> +	}
> +
> +	rh->ret_addr = *ret_addr;
> +	rh->frame = *frame;
> +
> +	/* Replace the return addr with trampoline addr. */
> +	*ret_addr = (unsigned long)arch_rethook_trampoline;
> +}
> +NOKPROBE_SYMBOL(arch_rethook_prepare);
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>