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=-0.8 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 75636C54FD0 for ; Mon, 20 Apr 2020 21:18:54 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 3BD8D207FC for ; Mon, 20 Apr 2020 21:18:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="jM7Fe9V9"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="XnbwyLaH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3BD8D207FC Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject: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=1fOIZ1cVGkIENqPNcV5g/UGASG2SDygEHnT/MIQAnSA=; b=jM7Fe9V9CWEpXZ P2MTSaFuoMnbRvsgAV4zxoih2fi2MU8VxLfTevQs3aHZSSevBaPkL7zgLTBCNIY+aRybMSa6037du 8nhc9OJb6QnLqBgGL93631LUPtmnrQdmVwc35SDrp9pf0sX+qX0fdMF1SRa5Q7elYxeeKojtku/wZ r4uFobZ6z3uuqJtlorKRsNRlhHB3w5fgKDHdMYumwdrOwQ461/IJRxg2wngMw3+VvxMONYO+p8EZt C6yj5EPqnA9rzUE43tao056j18mYSddloSs+ls9Qy0N+tu67RvXkIdcH9/YVkCCPHyqXnkpg4kTBC fAtJuAGQLPbUrcVhNtLQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jQdoY-0007UV-N3; Mon, 20 Apr 2020 21:18:50 +0000 Received: from mail-pg1-x543.google.com ([2607:f8b0:4864:20::543]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jQdoU-0007RN-Uh for linux-arm-kernel@lists.infradead.org; Mon, 20 Apr 2020 21:18:48 +0000 Received: by mail-pg1-x543.google.com with SMTP id n16so5712064pgb.7 for ; Mon, 20 Apr 2020 14:18:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=I17SvgP3468YaEZJ/m4ZeQYc9PSBmKeATPZO1LtBfTU=; b=XnbwyLaH7oxybL3ROw+62qMqb6U8ZOn3MLAaPYqeeh+VuVLkQNRb3nWFLiH9BwR2UR sbbVScuejmAiyvNPyxXICkWwFKWG0PpwnZEeN43Ngtm27+vb+YWt3zwPsRXvZuj689HF fYbSTVujfki3XSuRzMQMpiQebTqSfvME3FQtGM+9HbuL+7BEeSe0pUdrwZ07pFcg20Un 2m7XBjrFngOcMpgWWHxGDck8annUfzdJhxNT8BIU8kpWqrEkuSxsbuRyHwJPs71/4XUy QNd3JhuxImU8I1Q3PBefsRGrK1shSg18iwPgvpF5PlqpDJ18U2gU1CEsFxIrNQixCFNj B7uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=I17SvgP3468YaEZJ/m4ZeQYc9PSBmKeATPZO1LtBfTU=; b=OuNSdBGQ+TTG5aa9puedwybU/Wv04NyaQr9dWnPPb1ng8oDTkV1UpAQZHxQPNC371x dNXzpTJe6OISQntrxFtwRT7YkguSMJYfuyxfk1RK8NuflD8BJJrOj7wm7+nFqKSDrNvT t39Wp8ESA6AbCnUYmFx2+J/rvIA0Z70uPgbgVqI+YVTpQCJeGkIhsatpbZ86nn/3U/Kp Vl37ApR8WM+wsZB9fx6K7UV3iKV/vsz3DfiSHYqI6b9LoPN8Br9QKKI9x/vq2nQl5rVp 6lBHNL5a5h45N3IK9DtDGtzNwcre8r52ZwLQ6bBP+ZaXDaTZC0m/w+yCVFT9TyygEHmu urjQ== X-Gm-Message-State: AGi0PuabtDbiZhBEIANeRz/VAEfoIqByq4ByvuIdLtJk98oibD26FkDD xIDmUbFKS3APmFq7pFUsH+t3DQ== X-Google-Smtp-Source: APiQypLUjqh4gwB0agDoF7ZPn9MBLUchmrqjGKnG9TkryWTgi8fxU4nmUmPmgkJbrKA1PcXpLA4w9w== X-Received: by 2002:a63:ef02:: with SMTP id u2mr18271584pgh.21.1587417517829; Mon, 20 Apr 2020 14:18:37 -0700 (PDT) Received: from google.com ([2620:15c:201:2:ce90:ab18:83b0:619]) by smtp.gmail.com with ESMTPSA id s10sm407267pfd.124.2020.04.20.14.18.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2020 14:18:36 -0700 (PDT) Date: Mon, 20 Apr 2020 14:18:30 -0700 From: Sami Tolvanen To: Will Deacon Subject: Re: [PATCH v11 01/12] add support for Clang's Shadow Call Stack (SCS) Message-ID: <20200420211830.GA5081@google.com> References: <20191018161033.261971-1-samitolvanen@google.com> <20200416161245.148813-1-samitolvanen@google.com> <20200416161245.148813-2-samitolvanen@google.com> <20200420171727.GB24386@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200420171727.GB24386@willie-the-truck> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200420_141846_991131_32587E82 X-CRM114-Status: GOOD ( 31.84 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Juri Lelli , kernel-hardening@lists.openwall.com, Peter Zijlstra , Catalin Marinas , Marc Zyngier , Masahiro Yamada , clang-built-linux@googlegroups.com, Ingo Molnar , Laura Abbott , Dave Martin , Kees Cook , Jann Horn , Steven Rostedt , linux-arm-kernel@lists.infradead.org, Michal Marek , Ard Biesheuvel , Nick Desaulniers , linux-kernel@vger.kernel.org, Miguel Ojeda , James Morse , Masami Hiramatsu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Apr 20, 2020 at 06:17:28PM +0100, Will Deacon wrote: > > +ifdef CONFIG_SHADOW_CALL_STACK > > +CC_FLAGS_SCS := -fsanitize=shadow-call-stack > > +KBUILD_CFLAGS += $(CC_FLAGS_SCS) > > +export CC_FLAGS_SCS > > +endif > > CFLAGS_SCS would seem more natural to me, although I see ftrace does it this > way. Right, I followed ftrace's example here. > > +config SHADOW_CALL_STACK > > + bool "Clang Shadow Call Stack" > > + depends on ARCH_SUPPORTS_SHADOW_CALL_STACK > > + help > > + This option enables Clang's Shadow Call Stack, which uses a > > + shadow stack to protect function return addresses from being > > + overwritten by an attacker. More information can be found in > > + Clang's documentation: > > + > > + https://clang.llvm.org/docs/ShadowCallStack.html > > + > > + Note that security guarantees in the kernel differ from the ones > > + documented for user space. The kernel must store addresses of shadow > > + stacks used by other tasks and interrupt handlers in memory, which > > + means an attacker capable of reading and writing arbitrary memory > > + may be able to locate them and hijack control flow by modifying > > + shadow stacks that are not currently in use. > > Shouldn't some of this depend on CC_IS_CLANG? Sure, I'll add CC_IS_CLANG here in the next version. Note that we do check for compiler support before selecting ARCH_SUPPORTS_SHADOW_CALL_STACK. The flags are architecture-specific, so the check is done in the arch Kconfig. > > +config SHADOW_CALL_STACK_VMAP > > + bool "Use virtually mapped shadow call stacks" > > + depends on SHADOW_CALL_STACK > > + help > > + Use virtually mapped shadow call stacks. Selecting this option > > + provides better stack exhaustion protection, but increases per-thread > > + memory consumption as a full page is allocated for each shadow stack. > > Given that this feature applies only to arm64 kernels built with clang, it > feels weird to further segment that userbase with another config option. > Does Android enable SHADOW_CALL_STACK_VMAP? If not, maybe we should ditch > it for now and add it when we have a user. Android doesn't enable the VMAP option right now due to increased memory overhead. I'll drop it from v12. > > +/* > > + * A random number outside the kernel's virtual address space to mark the > > + * end of the shadow stack. > > + */ > > +#define SCS_END_MAGIC 0xaf0194819b1635f6UL > > This seems like it might be arm64-specific. Why not choose something based > off CONFIG_ILLEGAL_POINTER_VALUE (see linux/poison.h)? Sure, I'll use POISON_POINTER_DELTA here. > > +static inline void *__scs_base(struct task_struct *tsk) > > Please avoid using 'inline' in C files unless there's a good reason not > to let the compiler figure it out. Ack. > > +{ > > + /* > > + * To minimize risk the of exposure, architectures may clear a > > Should be "the risk of exposure". Thanks. > > + * The shadow call stack is aligned to SCS_SIZE, and grows > > + * upwards, so we can mask out the low bits to extract the base > > + * when the task is not running. > > + */ > > + return (void *)((unsigned long)task_scs(tsk) & ~(SCS_SIZE - 1)); > > Could we avoid forcing this alignment it we stored the SCS pointer as a > (base,offset) pair instead? That might be friendlier on the allocations > later on. The idea is to avoid storing the current task's shadow stack address in memory, which is why I would rather not store the base address either. > > +static inline void scs_set_magic(void *s) > > +{ > > + *scs_magic(s) = SCS_END_MAGIC; > > You added task_set_scs() for this sort of thing, so I'm not convinced you > need this extra helper. Agreed, I'll drop this. > > + if (s) > > + scs_set_magic(s); > > + /* TODO: poison for KASAN, unpoison in scs_free */ > > We don't usually commit these. What's missing? At the time, KASAN didn't support poisoning vmalloc'ed memory, but looks like that was fixed a while back. > > +static int scs_cleanup(unsigned int cpu) > > +{ > > + int i; > > + void **cache = per_cpu_ptr(scs_cache, cpu); > > + > > + for (i = 0; i < NR_CACHED_SCS; i++) { > > + vfree(cache[i]); > > + cache[i] = NULL; > > + } > > Hmm, can this run concurrently with another CPU doing a stack allocation > with this_cpu_cmpxchg()? It probably works out on arm64 thanks to the use > of atomics, but we shouldn't be relying on that in core code. This is essentially identical to the code in kernel/fork.c. Anyway, all of this code goes away with the VMAP option. > > +void __init scs_init(void) > > +{ > > + scs_cache = kmem_cache_create("scs_cache", SCS_SIZE, SCS_SIZE, > > + 0, NULL); > > + WARN_ON(!scs_cache); > > Memory allocation failure should be noisy enough without this. Sure, I'll remove the warning. > > +void scs_task_reset(struct task_struct *tsk) > > +{ > > + /* > > + * Reset the shadow stack to the base address in case the task > > + * is reused. > > + */ > > + task_set_scs(tsk, __scs_base(tsk)); > > +} > > Why isn't this in the header? > > +bool scs_corrupted(struct task_struct *tsk) > > +{ > > + unsigned long *magic = scs_magic(__scs_base(tsk)); > > + > > + return READ_ONCE_NOCHECK(*magic) != SCS_END_MAGIC; > > +} > > Same here. I'll move both to the header file. > > +void scs_release(struct task_struct *tsk) > > +{ > > + void *s; > > + > > + s = __scs_base(tsk); > > + if (!s) > > + return; > > + > > + WARN_ON(scs_corrupted(tsk)); > > + > > + task_set_scs(tsk, NULL); > > Aren't we about to free the task here? What does clearing the scs pointer > achieve? True, it doesn't achieve much, only leaves one fewer shadow stack pointer in memory. I'll drop this from the next version. Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel