From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 527E4C6FD1C for ; Wed, 22 Mar 2023 12:23:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=I9/GNXpXVEVYjMB5PgTQbz8xmlVEw//0UxyNTqBpx78=; b=GiBnLRv9sc++iy wbYEsjynGZDEOFmcv70VDUA4cQZcLwCjiuj+fHv16CmdOs0mrKHjZqOyuHVOc35uSrscQGlskJQd0 VkNs6U76fa9pADADAb5WWFITsQdyt1sTzRN2lhoN4bMU1rp+vXjuy71LmF+eaVdzN6U//ZEPcSO0b Q42eRTdD4mQYud0mzsNhozNt2uiVbYkOyr8DiM2OeSnp8PJSv+6EQsu782dmMW8tdMnvT9gfh/oSm VvwFwqJsN+jZHTkayqg7pxjY9HMuAMutzS7ALL53aT/l87QYrI7PbM4t6ELP55sgvJqXrsz15R00k ZlwhNXVFhmBGUKhDgCEg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pexTw-00Fuvy-1k; Wed, 22 Mar 2023 12:22:20 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pexTt-00FutZ-1A for linux-arm-kernel@lists.infradead.org; Wed, 22 Mar 2023 12:22:19 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9BBBD4B3; Wed, 22 Mar 2023 05:22:56 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.53.3]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5318C3F67D; Wed, 22 Mar 2023 05:22:10 -0700 (PDT) Date: Wed, 22 Mar 2023 12:22:07 +0000 From: Mark Rutland To: Josh Poimboeuf Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Peter Zijlstra , Jason Baron , Steven Rostedt , Ard Biesheuvel , Christophe Leroy , Paolo Bonzini , Sean Christopherson , Sami Tolvanen , Nick Desaulniers , Will McVicker , Kees Cook , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 08/11] arm64/static_call: Fix static call CFI violations Message-ID: References: <3d8c9e67a7e29f3bed4e44429d953e1ac9c6d5be.1679456900.git.jpoimboe@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3d8c9e67a7e29f3bed4e44429d953e1ac9c6d5be.1679456900.git.jpoimboe@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230322_052217_501417_38166629 X-CRM114-Status: GOOD ( 22.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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 Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel