| 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
| ||
|
Message-ID: <ZBry75KS3F+a0VM0@FVFF77S0Q05N>
Date: Wed, 22 Mar 2023 12:22:07 +0000
From: Mark Rutland <mark.rutland@....com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
Jason Baron <jbaron@...mai.com>,
Steven Rostedt <rostedt@...dmis.org>,
Ard Biesheuvel <ardb@...nel.org>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Sami Tolvanen <samitolvanen@...gle.com>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Will McVicker <willmcvicker@...gle.com>,
Kees Cook <keescook@...omium.org>,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 08/11] arm64/static_call: Fix static call CFI
violations
On Tue, Mar 21, 2023 at 09:00:14PM -0700, Josh Poimboeuf wrote:
> On arm64, with CONFIG_CFI_CLANG, it's trivial to trigger CFI violations
> by running "perf record -e sched:sched_switch -a":
>
> CFI failure at perf_misc_flags+0x34/0x70 (target: __static_call_return0+0x0/0xc; expected type: 0x837de525)
> WARNING: CPU: 3 PID: 32 at perf_misc_flags+0x34/0x70
> CPU: 3 PID: 32 Comm: ksoftirqd/3 Kdump: loaded Tainted: P 6.3.0-rc2 #8
> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> pstate: 904000c5 (NzcV daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : perf_misc_flags+0x34/0x70
> lr : perf_event_output_forward+0x74/0xf0
> sp : ffff80000a98b970
> x29: ffff80000a98b970 x28: ffff00077bd34d00 x27: ffff8000097d2d00
> x26: fffffbffeff6a360 x25: ffff800009835a30 x24: ffff0000c2e8dca0
> x23: 0000000000000000 x22: 0000000000000080 x21: ffff00077bd31610
> x20: ffff0000c2e8dca0 x19: ffff00077bd31610 x18: ffff800008cd52f0
> x17: 00000000837de525 x16: 0000000072923c8f x15: 000000000000b67e
> x14: 000000000178797d x13: 0000000000000004 x12: 0000000070b5b3a8
> x11: 0000000000000015 x10: 0000000000000048 x9 : ffff80000829e2b4
> x8 : ffff80000829c6f0 x7 : 0000000000000000 x6 : 0000000000000000
> x5 : fffffbffeff6a340 x4 : ffff00077bd31610 x3 : ffff00077bd31610
> x2 : ffff800009833400 x1 : 0000000000000000 x0 : ffff00077bd31610
> Call trace:
> perf_misc_flags+0x34/0x70
> perf_event_output_forward+0x74/0xf0
> __perf_event_overflow+0x12c/0x1e8
> perf_swevent_event+0x98/0x1a0
> perf_tp_event+0x140/0x558
> perf_trace_run_bpf_submit+0x88/0xc8
> perf_trace_sched_switch+0x160/0x19c
> __schedule+0xabc/0x153c
> dynamic_cond_resched+0x48/0x68
> run_ksoftirqd+0x3c/0x138
> smpboot_thread_fn+0x26c/0x2f8
> kthread+0x108/0x1c4
> ret_from_fork+0x10/0x20
>
> The problem is that the __perf_guest_state() static call does an
> indirect branch to __static_call_return0(), which isn't CFI-compliant.
IIUC that'd be broken even with the old CFI mechanism, since commit:
87b940a0675e2526 ("perf/core: Use static_call to optimize perf_guest_info_callbacks")
If so, we probably want a Fixes tag?
> Fix that by generating custom CFI-compliant ret0 functions for each
> defined static key.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
> arch/Kconfig | 4 ++
> arch/arm64/include/asm/static_call.h | 29 +++++++++++
> include/linux/static_call.h | 64 +++++++++++++++++++++----
> include/linux/static_call_types.h | 4 ++
> kernel/Makefile | 2 +-
> kernel/static_call.c | 2 +-
> tools/include/linux/static_call_types.h | 4 ++
> 7 files changed, 97 insertions(+), 12 deletions(-)
> create mode 100644 arch/arm64/include/asm/static_call.h
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e3511afbb7f2..8800fe80a0f9 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1348,6 +1348,10 @@ config HAVE_STATIC_CALL_INLINE
> depends on HAVE_STATIC_CALL
> select OBJTOOL
>
> +config CFI_WITHOUT_STATIC_CALL
> + def_bool y
> + depends on CFI_CLANG && !HAVE_STATIC_CALL
> +
> config HAVE_PREEMPT_DYNAMIC
> bool
>
> diff --git a/arch/arm64/include/asm/static_call.h b/arch/arm64/include/asm/static_call.h
> new file mode 100644
> index 000000000000..b3489cac7742
> --- /dev/null
> +++ b/arch/arm64/include/asm/static_call.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM64_STATIC_CALL_H
> +#define _ASM_ARM64_STATIC_CALL_H
> +
> +/*
> + * Make a dummy reference to a function pointer in C to force the compiler to
> + * emit a __kcfi_typeid_ symbol for asm to use.
> + */
> +#define GEN_CFI_SYM(func) \
> + static typeof(func) __used __section(".discard.cfi") *__UNIQUE_ID(cfi) = func
> +
> +
> +/* Generate a CFI-compliant static call NOP function */
> +#define __ARCH_DEFINE_STATIC_CALL_CFI(name, insns) \
> + asm(".align 4 \n" \
> + ".word __kcfi_typeid_" name " \n" \
> + ".globl " name " \n" \
> + name ": \n" \
> + "bti c \n" \
> + insns " \n" \
> + "ret \n" \
> + ".type " name ", @function \n" \
> + ".size " name ", . - " name " \n")
> +
> +#define __ARCH_DEFINE_STATIC_CALL_RET0_CFI(name) \
> + GEN_CFI_SYM(STATIC_CALL_RET0_CFI(name)); \
> + __ARCH_DEFINE_STATIC_CALL_CFI(STATIC_CALL_RET0_CFI_STR(name), "mov x0, xzr")
This looks correct, but given we're generating a regular functions it's
unfortunate we can't have the compiler generate the actual code with something
like:
#define __ARCH_DEFINE_STATIC_CALL_RET0_CFI(rettype, name, args...) \
rettype name(args) \
{ \
return (rettype)0; \
}
... but I guess passing the rettype and args around is painful.
Regardless, I gave this a spin atop v6.3-rc3 using LLVM 16.0.0 and CFI_CLANG,
and it does seem to work, so:
Tested-by: Mark Rutland <mark.rutland@....com>
Thanks,
Mark.
Powered by blists - more mailing lists