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 X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AC5D3C433E1 for ; Tue, 23 Jun 2020 09:40:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 74AF720776 for ; Tue, 23 Jun 2020 09:40:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 74AF720776 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id E609B6B0002; Tue, 23 Jun 2020 05:40:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E11466B0005; Tue, 23 Jun 2020 05:40:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D26726B0006; Tue, 23 Jun 2020 05:40:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0104.hostedemail.com [216.40.44.104]) by kanga.kvack.org (Postfix) with ESMTP id BB8F86B0002 for ; Tue, 23 Jun 2020 05:40:44 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4E21E8248047 for ; Tue, 23 Jun 2020 09:40:44 +0000 (UTC) X-FDA: 76959982008.13.cars10_05146e526e3a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin13.hostedemail.com (Postfix) with ESMTP id EFB1918140B67 for ; Tue, 23 Jun 2020 09:40:43 +0000 (UTC) X-HE-Tag: cars10_05146e526e3a X-Filterd-Recvd-Size: 5513 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf30.hostedemail.com (Postfix) with ESMTP for ; Tue, 23 Jun 2020 09:40:43 +0000 (UTC) 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 9776C1F1; Tue, 23 Jun 2020 02:40:42 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.20.203]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4D1203F6CF; Tue, 23 Jun 2020 02:40:39 -0700 (PDT) Date: Tue, 23 Jun 2020 10:40:36 +0100 From: Mark Rutland To: Kees Cook Cc: Thomas Gleixner , Elena Reshetova , x86@kernel.org, Andy Lutomirski , Peter Zijlstra , Catalin Marinas , Will Deacon , Alexander Potapenko , Alexander Popov , Ard Biesheuvel , Jann Horn , kernel-hardening@lists.openwall.com, linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 5/5] arm64: entry: Enable random_kstack_offset support Message-ID: <20200623094036.GD6374@C02TD0UTHF1T.local> References: <20200622193146.2985288-1-keescook@chromium.org> <20200622193146.2985288-6-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200622193146.2985288-6-keescook@chromium.org> X-Rspamd-Queue-Id: EFB1918140B67 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Jun 22, 2020 at 12:31:46PM -0700, Kees Cook wrote: > Allow for a randomized stack offset on a per-syscall basis, with roughly > 5 bits of entropy. > > In order to avoid unconditional stack canaries on syscall entry, also > downgrade from -fstack-protector-strong to -fstack-protector to avoid > triggering checks due to alloca(). Examining the resulting syscall.o, > sees no changes in canary coverage (none before, none now). Is there any way we can do this on invoke_syscall() specifically with an attribute? That'd help to keep all the concerns local in the file, and means that we wouldn't potentially lose protection for other functions that get added in future. > > Signed-off-by: Kees Cook > --- > arch/arm64/Kconfig | 1 + > arch/arm64/kernel/Makefile | 5 +++++ > arch/arm64/kernel/syscall.c | 10 ++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a4a094bedcb2..2902e5316e1a 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -135,6 +135,7 @@ config ARM64 > select HAVE_ARCH_MMAP_RND_BITS > select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT > select HAVE_ARCH_PREL32_RELOCATIONS > + select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET > select HAVE_ARCH_SECCOMP_FILTER > select HAVE_ARCH_STACKLEAK > select HAVE_ARCH_THREAD_STRUCT_WHITELIST > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index 151f28521f1e..39fc23d3770b 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -11,6 +11,11 @@ CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) > CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE) > CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) > > +# Downgrade to -fstack-protector to avoid triggering unneeded stack canary > +# checks due to randomize_kstack_offset. > +CFLAGS_REMOVE_syscall.o += -fstack-protector-strong > +CFLAGS_syscall.o += $(subst -fstack-protector-strong,-fstack-protector,$(filter -fstack-protector-strong,$(KBUILD_CFLAGS))) > + > # Object file lists. > obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ > entry-common.o entry-fpsimd.o process.o ptrace.o \ > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c > index 5f5b868292f5..00d3c84db9cd 100644 > --- a/arch/arm64/kernel/syscall.c > +++ b/arch/arm64/kernel/syscall.c > @@ -5,6 +5,7 @@ > #include > #include > #include > +#include > #include > > #include > @@ -42,6 +43,8 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, > { > long ret; > > + add_random_kstack_offset(); > + > if (scno < sc_nr) { > syscall_fn_t syscall_fn; > syscall_fn = syscall_table[array_index_nospec(scno, sc_nr)]; > @@ -51,6 +54,13 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno, > } > > regs->regs[0] = ret; > + > + /* > + * Since the compiler chooses a 4 bit alignment for the stack, > + * let's save one additional bit (9 total), which gets us up > + * near 5 bits of entropy. > + */ To explain the alignment requirement a bit better, how about: /* * The AAPCS mandates a 16-byte (i.e. 4-bit) aligned SP at * function boundaries. We want at least 5 bits of entropy so we * must randomize at least SP[8:4]. */ > + choose_random_kstack_offset(get_random_int() & 0x1FF); Do we have a rationale for randomizing bits SP[3:0]? If not, we might get better code gen with a 0x1F0 mask, since the compiler won't need to round down the SP. If we have a rationale that's fine, but we should spell it out more explicitly in the comment. Even if that's just "randomizing SP[3:0] isn't harmful, so we randomize those too". Thanks, Mark.