linux-kernel - Re: [PATCH 3/3] kbuild: propagate CONFIG_WERROR to resolve

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7764e9b-6254-42af-94b8-41562a18b58b@t-8ch.de>
Date: Mon, 25 Nov 2024 09:20:37 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Masahiro Yamada <masahiroy@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, 
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH 3/3] kbuild: propagate CONFIG_WERROR to resolve_btfids

On 2024-11-24 15:38:40-0800, Alexei Starovoitov wrote:
> On Sat, Nov 23, 2024 at 5:33 AM Thomas Weißschuh <linux@...ssschuh.net> wrote:
> >
> > Use CONFIG_WERROR to also fail on warnings emitted by resolve_btfids.
> > Allow the CI bots to prevent the introduction of new warnings.
> >
> > Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> > ---
> >  scripts/link-vmlinux.sh | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index a9b3f34a78d2cd4514e73a728f1a784eee891768..61f1f670291351a276221153146d66001eca556c 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -274,7 +274,11 @@ vmlinux_link vmlinux
> >  # fill in BTF IDs
> >  if is_enabled CONFIG_DEBUG_INFO_BTF; then
> >         info BTFIDS vmlinux
> > -       ${RESOLVE_BTFIDS} vmlinux
> > +       RESOLVE_BTFIDS_ARGS=""
> > +       if is_enabled CONFIG_WERROR; then
> > +               RESOLVE_BTFIDS_ARGS=" --fatal-warnings "
> > +       fi
> > +       ${RESOLVE_BTFIDS} ${RESOLVE_BTFIDS_ARGS} vmlinux
> 
> I'm not convinced we need to fail the build when functions are renamed.
> These warns are eventually found and fixed.

The same could be said for most other build warnings.
CONFIG_WERROR is a well-known opt-in switch for exactly this behavior.

Fixing these warnings before they hit mainline has various
advantages. The author introducing the warning knows about the full
impact of their change, discussions can be had when everybody still
has the topic fresh on their mind and other unrelated people don't get
confused, like me or [0].

The "eventually fixed" part seems to have been me the last two times :-)

Given the fairly simple implementation, in my opinion this is worth doing.

Please note that I have two fairly trivial changes for a v2 and would
also like to get some feedback from Masahiro, especially for patch 1.


Thomas

[0] https://lore.kernel.org/lkml/20241113093703.9936-1-laura.nao@collabora.com/