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=ham 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 41B10C433E0 for ; Tue, 23 Jun 2020 09:40:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 28A9D20774 for ; Tue, 23 Jun 2020 09:40:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732099AbgFWJkn (ORCPT ); Tue, 23 Jun 2020 05:40:43 -0400 Received: from foss.arm.com ([217.140.110.172]:47334 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731786AbgFWJkn (ORCPT ); Tue, 23 Jun 2020 05:40:43 -0400 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> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.