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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 15 Mar 2022 19:08:48 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        "Naveen N . Rao" <naveen.n.rao@...ux.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v11 07/12] ARM: rethook: Add rethook arm implementation

On Sat, 12 Mar 2022 01:00:03 +0900
Masami Hiramatsu <mhiramat@...nel.org> 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@intel.com/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@...nel.org>