Received: by 2002:a05:6902:102b:0:0:0:0 with SMTP id x11csp2293683ybt; Tue, 16 Jun 2020 02:10:56 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxv0OolnARrGnyeUCzBbhHSBF6+HpCo8ygaYCqvJqHuDFPftPzVzXPFfs3dtHz5iLAhXfxm X-Received: by 2002:a17:906:6890:: with SMTP id n16mr1754964ejr.553.1592298656125; Tue, 16 Jun 2020 02:10:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1592298656; cv=none; d=google.com; s=arc-20160816; b=SbpdcyrUF1fMRyYYM7Rpo/m7GLcqz1poWVMMoxDNWbTC3Rn4JH2MPMM1zAE1ThaDH2 msyHyyqWtMWcso8kWw6bGekAjU11mVon0VGESm2AixiWyMS5BguWdkXbdZhmXFjM6XOM 9YUpRg2DUupC9NFLRuHD88EPE78dAycj1Biy2zabzBbQVNyczmaZ5K1avo1op3limcNy byv990fXpfECpGJj6pEURRuwlCxRTbl8zWE905O2m8a9HwSZB3pn/ZcRJgq20BqmNaax FjsHc5NUDef84GwXJC+ArGZQD2gAvKi2pHfbcX7UKiG1tsyH19DAkbhJKDeDS4sMS5MV QHSA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=Eu2oq0yjr6Eq5I9NRCK1BJJMT0oPPDxPZXG6oIi1TI0=; b=Yet7R+ye1maECX9nNJEzttXbmyu5/e2SdGhswQifOwMODsnWG0uKKQ8i219jrYQmqM El3LqMOMHGW8RlIwnzHBhMClyG5J5W4+zL0ETPQEHeFy2lbBNYsxTGGfc1N6fElVp+PM 2RiMaPwPGO6kX+XrVyBzxccILoLBYexk/pcu3I3zxnunvfBQzL2Zqq1z3EaQPy89U3Xf DYkEDNmiXzCTZjUmsQB+qqPWS1DwhBfZJRPBKbe7tjyCL2yp7vNoudluCPH1OKldkWPK N5ZH6EDHOI6VGF+TTIS593FfWVgsBox87+XbIGEyhGp3EvhTaJTmiZpOEd1y8itVQRAN 2zwQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=ezt4NfB4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id q5si2219578ejb.751.2020.06.16.02.10.32; Tue, 16 Jun 2020 02:10:56 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=ezt4NfB4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726573AbgFPJIM (ORCPT + 99 others); Tue, 16 Jun 2020 05:08:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725710AbgFPJIL (ORCPT ); Tue, 16 Jun 2020 05:08:11 -0400 Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69BCFC05BD43 for ; Tue, 16 Jun 2020 02:08:11 -0700 (PDT) Received: by mail-pf1-x442.google.com with SMTP id d66so9198043pfd.6 for ; Tue, 16 Jun 2020 02:08:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Eu2oq0yjr6Eq5I9NRCK1BJJMT0oPPDxPZXG6oIi1TI0=; b=ezt4NfB4JG+xSwwNShZEsNSM9PWNfoR57G0ueDUBHNAzAqMLXb6Kft8YjHXYBqLP4f 87N1XLEK4GD+Q3c1r7MUl0tiKmcWsW+4HnCO5HAm3Ut0a8IivFBhk6eBibNilpHhqaxG HT9HIss0WFfQ5nn45hXP0J1caAdgZEkzMS660= 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=Eu2oq0yjr6Eq5I9NRCK1BJJMT0oPPDxPZXG6oIi1TI0=; b=j43fTjelw9PcvpGq4qmlYV0YgGfTyAvAe3I6vWnlD2wtrRHOCc/Q8AooF8wrssMCDY gbMaM5CgNcnPInGa9tyw3JAiF3sj28jCaYaW4U7Hq5gqQdPif+z4wsah7cgJdAf1XfcE N0Q6rdXsRYjl9NB58sLIynVkhPqXvj1vwRWQLUNTLjfl0APEpz5QLNOL+faqMT47dgU5 f0/4reMN7RhZ8vKtYODZ2mle6MAkUSF5jhVV4OPL1Vzm6+ZnIADL6EJgDIurcJ2MlGAn kfwRazW/rUXNE5/kVC9w68hQNRkmDG6yJU1OPj1nCsnHw7ZFF68dVfrqkF/0MiUygBB5 q3IQ== X-Gm-Message-State: AOAM530Ba1SRVJZMLhulBW66fI+TV5U+KbgiEHkMBTjDdqQq81ksIvNz xrwKsrh9zw6BJgmEFg4QchvHEg== X-Received: by 2002:a62:3645:: with SMTP id d66mr1252372pfa.275.1592298490417; Tue, 16 Jun 2020 02:08:10 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id 6sm16449421pfi.170.2020.06.16.02.08.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jun 2020 02:08:09 -0700 (PDT) Date: Tue, 16 Jun 2020 02:08:08 -0700 From: Kees Cook To: glider@google.com Cc: yamada.masahiro@socionext.com, jmorris@namei.org, maze@google.com, ndesaulniers@google.com, gregkh@linuxfoundation.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] [RFC] security: allow using Clang's zero initialization for stack variables Message-ID: <202006160207.E9C6FDDB7@keescook> References: <20200616083435.223038-1-glider@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200616083435.223038-1-glider@google.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 16, 2020 at 10:34:35AM +0200, glider@google.com wrote: > In addition to -ftrivial-auto-var-init=pattern (used by > CONFIG_INIT_STACK_ALL now) Clang also supports zero initialization for > locals enabled by -ftrivial-auto-var-init=zero. > The future of this flag is still being debated, see > https://bugs.llvm.org/show_bug.cgi?id=45497 > Right now it is guarded by another flag, > -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang, > which means it may not be supported by future Clang releases. > Another possible resolution is that -ftrivial-auto-var-init=zero will > persist (as certain users have already started depending on it), but the > name of the guard flag will change. > > In the meantime, zero initialization has proven itself as a good > production mitigation measure against uninitialized locals. Unlike > pattern initialization, which has a higher chance of triggering existing > bugs, zero initialization provides safe defaults for strings, pointers, > indexes, and sizes. On the other hand, pattern initialization remains > safer for return values. > Performance-wise, the difference between pattern and zero initialization > is usually negligible, although the generated code for zero > initialization is more compact. > > This patch renames CONFIG_INIT_STACK_ALL to > CONFIG_INIT_STACK_ALL_PATTERN and introduces another config option, > CONFIG_INIT_STACK_ALL_ZERO, that enables zero initialization for locals > if the corresponding flags are supported by Clang. > > Cc: Kees Cook > Cc: Nick Desaulniers > Cc: Greg Kroah-Hartman > Signed-off-by: Alexander Potapenko Thanks! I've applied this to my for-next/kspp tree (with a few small tweaks). > -- ^^ note, this separator should be "---" for diff tools to do the right thing, etc. > v2: > - as suggested by Kees Cook, make CONFIG_INIT_STACK_ALL_PATTERN and > CONFIG_INIT_STACK_ALL_ZERO separate options. > --- > Makefile | 12 ++++++++++-- > init/main.c | 6 ++++-- > security/Kconfig.hardening | 29 +++++++++++++++++++++++++---- > 3 files changed, 39 insertions(+), 8 deletions(-) > > diff --git a/Makefile b/Makefile > index fd31992bf918..fa739995ee12 100644 > --- a/Makefile > +++ b/Makefile > @@ -802,11 +802,19 @@ KBUILD_CFLAGS += -fomit-frame-pointer > endif > endif > > -# Initialize all stack variables with a pattern, if desired. > -ifdef CONFIG_INIT_STACK_ALL > +# Initialize all stack variables with a 0xAA pattern. > +ifdef CONFIG_INIT_STACK_ALL_PATTERN > KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern > endif > > +# Initialize all stack variables with a zero pattern. > +ifdef CONFIG_INIT_STACK_ALL_ZERO > +# Future support for zero initialization is still being debated, see > +# https://bugs.llvm.org/show_bug.cgi?id=45497. These flags are subject to being > +# renamed or dropped. > +KBUILD_CFLAGS += -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang > +endif > + > DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments) > > ifdef CONFIG_DEBUG_INFO > diff --git a/init/main.c b/init/main.c > index 0ead83e86b5a..ee08cef4aa1a 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -779,8 +779,10 @@ static void __init report_meminit(void) > { > const char *stack; > > - if (IS_ENABLED(CONFIG_INIT_STACK_ALL)) > - stack = "all"; > + if (IS_ENABLED(CONFIG_INIT_STACK_ALL_PATTERN)) > + stack = "all (pattern)"; > + else if (IS_ENABLED(CONFIG_INIT_STACK_ALL_ZERO)) > + stack = "all (zero)"; > else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)) > stack = "byref_all"; > else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF)) > diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening > index af4c979b38ee..7b705611ccaa 100644 > --- a/security/Kconfig.hardening > +++ b/security/Kconfig.hardening > @@ -19,13 +19,16 @@ config GCC_PLUGIN_STRUCTLEAK > > menu "Memory initialization" > > -config CC_HAS_AUTO_VAR_INIT > +config CC_HAS_AUTO_VAR_INIT_PATTERN > def_bool $(cc-option,-ftrivial-auto-var-init=pattern) > > +config CC_HAS_AUTO_VAR_INIT_ZERO > + def_bool $(cc-option,-ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang) > + > choice > prompt "Initialize kernel stack variables at function entry" > default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS > - default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT > + default INIT_STACK_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN > default INIT_STACK_NONE > help > This option enables initialization of stack variables at > @@ -88,15 +91,33 @@ choice > of uninitialized stack variable exploits and information > exposures. > > - config INIT_STACK_ALL > + config INIT_STACK_ALL_PATTERN > bool "0xAA-init everything on the stack (strongest)" > - depends on CC_HAS_AUTO_VAR_INIT > + depends on CC_HAS_AUTO_VAR_INIT_PATTERN > help > Initializes everything on the stack with a 0xAA > pattern. This is intended to eliminate all classes > of uninitialized stack variable exploits and information > exposures, even variables that were warned to have been > left uninitialized. > + Pattern initialization is known to provoke many existing bugs > + related to uninitialized locals, e.g. pointers receive > + non-NULL values, buffer sizes and indices are very big. > + > + config INIT_STACK_ALL_ZERO > + bool "zero-init everything on the stack (strongest and safest)" > + depends on CC_HAS_AUTO_VAR_INIT_ZERO > + help > + Initializes everything on the stack with a zero > + pattern. This is intended to eliminate all classes > + of uninitialized stack variable exploits and information > + exposures, even variables that were warned to have been > + left uninitialized. > + Zero initialization provides safe defaults for strings, > + pointers, indices and sizes, and is therefore more suitable as > + a security mitigation measure. > + The corresponding flag isn't officially supported by Clang and > + may sooner or later go away or get renamed. > > endchoice > > -- > 2.27.0.290.gba653c62da-goog > -- Kees Cook