Date:   Mon, 6 Dec 2021 11:53:41 -0800
From:   Kees Cook <keescook@chromium.org>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Salvatore Bonaccorso <carnil@debian.org>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to
 gcc
Message-ID: <202112061128.6B670358@keescook>
References: <YatpectAYsWnmPy2@eldamar.lan>
 <CAHk-=whTTWUyL5j5_-UeRT6k9VcJM_VOfjiKuU2NBJkxhbnXpw@mail.gmail.com>
 <CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com>
Precedence: bulk

On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > >
> > > Andreas suggested to replace the
> > >
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > >
> > > with
> > >
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> >
> > Ugh. I think the external build environment is a bit broken, but
> > whatever. The above is ugly but I guess it works.
> >
> > Another alternative would be to make the Kconfig strings simply not
> > have '"' as part of them.
> >
> > When you do
> >
> >     a = "hello"
> >     print $a
> >
> > in any normal language, you generally wouldn't expect it to print the
> > quotes, it should just print the bare word.
> >
> > But that's what the Kconfig string language basically does in this
> > case. And I guess several users expect and take advantage of that ;(
> >
> > Masahiro? Comments?
> 
> Yes, you get to the point.
> 
> In fact, this is in my TODO list for a while
> (and this is the reason I was doing prerequisite Kconfig refactoring
> in the previous development cycle).
> I will try to find some spare time to complete this work.
> 
> 
> 
> Kconfig generates two similar files,
> 
>  -   .config
>  -   include/config/auto.conf
> 
> Changing the format of the .config is presumably problematic
> since it is the saved user configuration as well.
> 
> It is possible (and more reasonable) to change include/config/auto.conf
> so strings are not quoted.
> 
> In Makefiles, quotations are just normal characters; they have no
> special functionality.
> 
> So, in Makefile context, it is more handy to do
> 
>      CONFIG_X=foo bar
> 
> instead of
> 
>     CONFIG_X="foo bar"
> 
> 
> 
> One problem is include/config/auto.conf is included not only by Makefiles
> but also by shell scripts.
> 
> 
> In shell context, the right hand side must be quoted
> in case the value contains spaces.
> 
>    CONFIG_X="foo bar"
> 
> 
> 
> My plan is to fix
>   scripts/link-vmlinux.sh
>   scripts/gen_autoksyms.sh
> etc. to not directly include the auto.conf.
> Later, change Kconfig to generate the auto.conf without "".
> 
> 
> 
> In the meantime,
> 
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> 
>  or if you prefer slightly shorter form,
> 
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> 
> will be a workaround.

It'll be nice to get this fixed. There are a few places where there is
a test for a compiler flag in Kconfig, and then the option is repeated
in the Makefile, due to the above quoting issues. For example:

arch/arm64/Kconfig:
	config CC_HAS_BRANCH_PROT_PAC_RET
	     # GCC 9 or later, clang 8 or later
	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)

arch/arm64/Makefile:
	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf


I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.

-- 
Kees Cook