[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y02zWFxC92VDSpdZ@dev-arch.thelio-3990X>
Date: Mon, 17 Oct 2022 12:56:08 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: "Li, Xin3" <xin3.li@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"H.Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>, llvm@...ts.linux.dev,
linux-kbuild@...r.kernel.org
Subject: Re: upgrade the orphan section warning to a hard link error
On Mon, Oct 17, 2022 at 12:32:39PM -0700, Kees Cook wrote:
> On Mon, Oct 17, 2022 at 11:26:47AM -0700, Nathan Chancellor wrote:
> > It might be interesting to turn orphan sections into an error if
> > CONFIG_WERROR is set. Perhaps something like the following (FYI, not
> > even compile tested)?
> >
> > diff --git a/Makefile b/Makefile
> > index 0837445110fc..485f47fc2c07 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1119,7 +1119,7 @@ endif
> > # We never want expected sections to be placed heuristically by the
> > # linker. All sections should be explicitly named in the linker script.
> > ifdef CONFIG_LD_ORPHAN_WARN
> > -LDFLAGS_vmlinux += --orphan-handling=warn
> > +LDFLAGS_vmlinux += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
> > endif
>
> Yes, this is much preferred.
>
> > Outright turning the warning into an error with no escape hatch might be
> > too aggressive, as we have had these warnings triggered by new compiler
> > generated sections, such as in commit 848378812e40 ("vmlinux.lds.h:
> > Handle clang's module.{c,d}tor sections"). Unconditionally breaking the
> > build in these situations is unfortunate but the warnings do need to be
> > dealt with so I think having it error by default with the ability to
> > opt-out is probably worth doing. I do not have a strong opinion though.
>
> Correct; the mandate from Linus (disregarding his addition of
> CONFIG_WERROR for all*config builds), is that we should avoid breaking
> builds. It wrecks bisection, it causes problems across compiler versions,
> etc.
>
> So, yes, only on CONFIG_WERROR=y.
We would probably want to alter the text of CONFIG_WERROR in some manner
to convey this, perhaps like so:
diff --git a/init/Kconfig b/init/Kconfig
index a19314933e54..1fc03e4b2af2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -165,10 +165,12 @@ config WERROR
help
A kernel build should not cause any compiler warnings, and this
enables the '-Werror' (for C) and '-Dwarnings' (for Rust) flags
- to enforce that rule by default.
+ to enforce that rule by default. Certain warnings from other tools
+ such as the linker may be upgraded to errors with this option as
+ well.
- However, if you have a new (or very old) compiler with odd and
- unusual warnings, or you have some architecture with problems,
+ However, if you have a new (or very old) compiler or linker with odd
+ and unusual warnings, or you have some architecture with problems,
you may need to disable this config option in order to
successfully build the kernel.
---
Cheers,
Nathan
Powered by blists - more mailing lists