Received: by 10.223.185.116 with SMTP id b49csp2063917wrg; Mon, 12 Feb 2018 03:46:43 -0800 (PST) X-Google-Smtp-Source: AH8x226pGE4RuiH8wMCeU4nU3HsAtxCUtr1GlKCuw3PKix1PDpeoIkpDJJWtJhgoA+bLSYk07tkN X-Received: by 10.98.34.15 with SMTP id i15mr11365284pfi.168.1518436003841; Mon, 12 Feb 2018 03:46:43 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1518436003; cv=none; d=google.com; s=arc-20160816; b=klErRoo2Er13MTB9ROgt87XaSiaRw4ziRupGaLO5hNEL/ogvgjdj9rMSJEUAR+EUNr u6cuwoSEADU3FVH4AjrtlvUOoP5ril9nl9SPBqW4eLkNe/bmym1n4NJ92CamoWp93fBv reFTGPkqmXQDoqsZ2M8IlUe7nb9fh78TaUhDuKNfzTLOPolyrrgvU8VIS57nPASkFbvA yDzUpgcs6VzuIZr1fO0yQmcqcvOzZF8mPeIB1Aw+CTS6nFhUeZ+63Vvp1FY600+WzeNa 5KSxoXDob9r7lo/NT15zSJYADi2qdYinZY2Nu15gLze/32jtjAp1HrYeZb17jQSyDnxP /FBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=ZhMxCDl8YSVIxWg1F3e3K9axma0q5LIzKRBSqAF7m4c=; b=Y5NjmB7t4yfSPdNVeKJeaJ/qjpytcQTes6YvK11MD7k3sWDZw5UahR8NG6gdrEJMoW OjzIbPTf4NdqZBwfdMdYjHrojdjlfhfrklyWt7K75CXsZ8mgdAiiSchv3zagTG6j3zGr 1+Rc72cQOT2WM9eC/LX0noHIRDj7AXoa5gZjjunYTB1VRlF/4hueh2+VtddzexquMtsV X7Hxp5E/Ceb/ArzZiuoG4y+4A7RMo+2QlmYsUEzpCCqQGNsGFEaZZ5541czfo59uqQjP vD6I73+xpXN/kwwIkeA+SRSMZ1Jp21+SvdZjsmsqYEPGjMTBgY8Vj+oVnmKOYyuJCoGU syEA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ZMzsIu3G; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id e17si2627880pgt.406.2018.02.12.03.46.28; Mon, 12 Feb 2018 03:46:43 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ZMzsIu3G; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934179AbeBLLpE (ORCPT + 99 others); Mon, 12 Feb 2018 06:45:04 -0500 Received: from mail-vk0-f45.google.com ([209.85.213.45]:45916 "EHLO mail-vk0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934038AbeBLLpC (ORCPT ); Mon, 12 Feb 2018 06:45:02 -0500 Received: by mail-vk0-f45.google.com with SMTP id j204so8607777vke.12; Mon, 12 Feb 2018 03:45:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ZhMxCDl8YSVIxWg1F3e3K9axma0q5LIzKRBSqAF7m4c=; b=ZMzsIu3GCSFiRLYok3poxpF69mRZk8S1abLF1zA/0aDxVdOh0Jd0Qe6iTLvrkJVQNA AfKqTCfTWgfj19R5lk2tpDGwQKHNwTJKe0UYiuYAN+QbB2ugbeYlV5rHN4QRMkYpU4iF NVtkeXBGQUhppzNtxQTwzkFcnpHJpD/A/RYgoJ3I9S1PceHQNj/UgUPtEGqvWn5/djD9 F0leAiLngcP+KX0gD+Kx79IaLP6JCXYJSjwaA8Iuu1ZJTULPMCU1S2cDjSl84oentrJW HucVaBbvCLZa9HGnjYnQMjxhz99JDh0fquj9YKR8smxKT1mD1ezWiNOvsKYZSOxRh/pf qgvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ZhMxCDl8YSVIxWg1F3e3K9axma0q5LIzKRBSqAF7m4c=; b=krSWFK1CLu/LsQC0KnWv+VHgHDDw+nVl6C1P5mhr7/IDcbF6WWa3ZQiBXIKQjLPOf0 sJFRVPYHIEbg+l6Ag52I1AoCbvnRns77BR/aW4XHe3rCuC5iSjvyx1YJSsw+kBJup4Ya QhYK8K5/6nDhQmfgKc6vwCzOFegP+Dwpk54GnoRhovcjAzB6Jm5hSevE1zHmXIfvN2zv Yj08svAi8w36V6ArOqMxvyX7NE1wHkBpSFEghGBMzw/nLYwoCcXY7frN4qD6ek5/fKE1 2WGVAgiOKouuaILeyCv1UFNsfcAJIQ0yREKNkotGxh3bP7hrGe1iA1kMesL4hOrynD5Q N0NA== X-Gm-Message-State: APf1xPDhk6b8yM7luKQ37qXW3fjjGxrB5wWozsyog+TAz73p6jdUKUG8 53eJbbuCmwZJyARU2BO2FSvr9Y56lHP+2Eud9i4= X-Received: by 10.31.181.69 with SMTP id e66mr10202064vkf.122.1518435900963; Mon, 12 Feb 2018 03:45:00 -0800 (PST) MIME-Version: 1.0 Received: by 10.103.70.21 with HTTP; Mon, 12 Feb 2018 03:44:59 -0800 (PST) In-Reply-To: References: <20180210054843.z3g7wvcmlccvww3h@huvuddator> <20180210074924.3nhxsza5zdbaahxx@huvuddator> <20180210080556.mycqsjhxbaguwhay@huvuddator> <20180210085519.737ckf4bcl57h4g2@huvuddator> <20180211103432.pf2ot6nd7nbhdhsy@huvuddator> From: Ulf Magnusson Date: Mon, 12 Feb 2018 12:44:59 +0100 Message-ID: Subject: Re: [RFC PATCH 4/7] kconfig: support new special property shell= To: Masahiro Yamada Cc: Linus Torvalds , Kees Cook , Linux Kbuild mailing list , Greg Kroah-Hartman , Andrew Morton , Nicolas Pitre , "Luis R . Rodriguez" , Randy Dunlap , Sam Ravnborg , Michal Marek , Martin Schwidefsky , Pavel Machek , linux-s390 , Jiri Kosina , Linux Kernel Mailing List , Tejun Heo , Ingo Molnar , "Van De Ven, Arjan" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada wrote: > 2018-02-11 19:34 GMT+09:00 Ulf Magnusson : >> Looks to me like there's a few unrelated issues here: >> >> >> 1. The stack protector support test scripts >> >> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or a >> subtly broken kernel from being built. >> >> A few questions: >> >> - How do things fail with a broken stack protector implementation? >> >> - How common are those broken compilers? >> >> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for breakage, >> or would a simpler static test work in practice? >> >> I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into >> Kconfig, but should make sure it's actually needed in any case. >> >> The scripts are already split up as >> >> scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh >> >> by the way, though only gcc-x86_32-has-stack-protector.sh and >> gcc-x86_64-has-stack-protector.sh exist. >> >> - How old do you need to go with GCC for -fno-stack-protector to give an >> error (i.e., for not even the option to be recognized)? Is it still >> warranted to test for it? >> >> Adding some CCs who worked on the stack protector test scripts. >> >> And yeah, I was assuming that needing support scripts would be rare, and that >> you'd usually just check whether gcc accepts the flag. >> >> When you Google "gcc broken stack protector", the top hits about are about the >> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a false >> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add >> -fno-PIE")). >> >> >> 2. Whether to hide the Kconfig stack protector alternatives or always show them >> >> Or equivalently, whether to automatically fall back on other stack protector >> alternatives (including no stack protector) if the one specified in the .config >> isn't available. >> >> I'll let you battle this one out. In any case, as a user, I'd want a >> super-clear message telling me what to change if the build breaks because of >> missing stack protector support. >> >> >> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles >> >> I'd just go with whatever is simplest here. I don't find the Kconfig version >> too bad, but I'm already very familiar with Kconfig, so it's harder for me to >> tell how it looks to other people. >> >> I'd add some comments to explain the idea in the final version. >> >> @Kees: >> I was assuming that the Makefiles would error out with a message if none of the >> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning. >> >> You could offload part of that check to Kconfig with something like >> >> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE >> def_bool CC_STACKPROTECTOR_STRONG || \ >> CC_STACKPROTECTOR_REGULAR || \ >> CC_STACKPROTECTOR_NONE >> >> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile. >> It has the advantage of making the constraint clear in the Kconfig file >> at least. >> >> You could add some kind of assert feature to Kconfig too, but IMO it's not >> warranted purely for one-offs like this at least. >> >> That's details though. I'd want to explain it with a comment in any case if we >> go with something like this, since it's slightly kludgy and subtle >> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you can't >> express it like that directly, since it's derived from other symbols). >> >> >> Here's an overview of the current Kconfig layout by the way, assuming >> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being >> implemented in Kconfig: >> >> # Feature tests >> CC_HAS_STACKPROTECTOR_STRONG >> CC_HAS_STACKPROTECTOR_REGULAR >> CC_HAS_STACKPROTECTOR_NONE >> >> # User request >> WANT_CC_STACKPROTECTOR_AUTO >> WANT_CC_STACKPROTECTOR_STRONG >> WANT_CC_STACKPROTECTOR_REGULAR >> WANT_CC_STACKPROTECTOR_NONE >> >> # The actual "output" to the Makefiles >> CC_STACKPROTECTOR_STRONG >> CC_STACKPROTECTOR_REGULAR >> CC_STACKPROTECTOR_NONE >> >> # Some possible output "nicities" >> CHOSEN_CC_STACKPROTECTOR_AVAILABLE >> CC_OPT_STACKPROTECTOR >> >> Does anyone have objections to the naming or other things? I saw some >> references to "Santa's wish list" in messages of commits that dealt with other >> variables named WANT_*, though I didn't look into those cases. ;) >> > > > > I think Linus's comment was dismissed here. > > > Linus said: > >> But yes, I also reacted to your earlier " It can't silently rewrite it >> to _REGULAR because the compiler support for _STRONG regressed." >> Because it damn well can. If the compiler doesn't support >> -fstack-protector-strong, we can just fall back on -fstack-protector. >> Silently. No extra crazy complex logic for that either. > > > If I understood his comment correctly, > we do not need either WANT_* or _AUTO. > > > > > Kees' comment: > >> In the stack-protector case, this becomes quite important, since the >> goal is to record the user's selection regardless of compiler >> capability. For example, if someone selects _REGULAR, it shouldn't >> "upgrade" to _STRONG. (Similarly for _NONE.) > > No. Kconfig will not do this silently. (Playing devil's advocate...) If the user selected _STRONG and it becomes unavailable later, it seems to silently fall back on other options, even for oldnoconfig (which just checks if there are any new symbols in the choice). It would be possible to also check if the old user selection still applies btw. I do that in Kconfiglib. It's arguable whether that matches the intent of oldnoconfig. > > > "make oldconfig" (or "make silentoldconfig" as well) > always ask users about new symbols. > > > For example, let's say your compiler supports -fstack-protector > but not -fstack-protector-strong. > CC_STACKPROTECTOR_REGULAR is the best you can choose. > > Then, let's say you upgrade your compiler and the new compiler supports > -fstack-protector-strong as well. > > In this case, CC_STACKPROTECTOR_STRONG is a newly visible symbol, > so Kconfig will ask you > "Hey, you were previously using _REGULAR, but your new compiler > also supports _STRONG. Do you want to use it?" > > > The "make oldconfig" will look like follows: > > > masahiro@grover:~/workspace/linux-kbuild$ make oldconfig > HOSTCC scripts/kconfig/conf.o > HOSTLD scripts/kconfig/conf > scripts/kconfig/conf --oldconfig Kconfig > * > * Restart config... > * > * > * Linux Kernel Configuration > * > Stack Protector buffer overflow detection > 1. Strong (CC_STACKPROTECTOR_STRONG) (NEW) >> 2. Regular (CC_STACKPROTECTOR_REGULAR) > 3. None (CC_STACKPROTECTOR_NONE) > choice[1-3?]: > > > > Please notice the Strong is marked as "(NEW)". > > Kconfig handles this really nicely with Linus' suggestion. > > [1] Users can select only features supported by your compiler > - this makes sense. > > [2] If you upgrade your compiler and it provides more capability, > "make (silent)oldconfig" will ask you about new choices. > - this also makes sense. If you switch to a compiler that provides less capability, it'll silently forget the old user preference though. > > > > So, the following simple implementation works well enough, > doesn't it? > > ---------------->8--------------- > choice > prompt "Stack Protector buffer overflow detection" > > config CC_STACKPROTECTOR_STRONG > bool "Strong" > depends on CC_HAS_STACKPROTECTOR_STRONG > select CC_STACKPROTECTOR > > config CC_STACKPROTECTOR_REGULAR > bool "Regular" > depends on CC_HAS_STACKPROTECTOR > select CC_STACKPROTECTOR > > config CC_STACKPROTECTOR_NONE > bool "None" > > endchoice > ---------------->8------------------------- Obviously much neater at least. :) > > > > BTW, do we need to use 'choice' ? > > > The problem of using 'choice' is, > it does not work well with allnoconfig. > > > For all{yes,mod}config, we want to enable as many features as possible. > For allnoconfig, we want to disable as many as possible. Oh... right. For allnoconfig, it would always pick the default, so if the default is "on", it's bad there. > > However, the default of 'choice' is always the first visible value. > > > So, I can suggest to remove _REGULAR and _NONE. > > We have just two bool options, like follows. > > ------------------->8----------------- > config CC_STACKPROTECTOR > bool "Use stack protector" > depends on CC_HAS_STACKPROTECTOR > > config CC_STACKPROTECTOR_STRONG > bool "Use strong strong stack protector" > depends on CC_STACKPROTECTOR > depends on CC_HAS_STACKPROTECTOR_STRONG > -------------------->8------------------ Even neater. Much easier to understand. > > This will work well for all{yes,mod,no}config. > > We will not have a case where > -fstack-protector-strong is supported, but -fstack-protector is not. > > > -- > Best Regards > Masahiro Yamada So there's just the caveat about possibly "forgetting" the user preferences and automatically falling back on CC_STACKPROTECTOR_REGULAR or CC_STACKPROTECTOR_NONE when the environment changes, instead of erroring out. When you have to think hard about cases where something might break, it might be a sign that it's not a huge problem in practice, but I can't really speak for the priorities here. Cheers, Ulf