linux-kernel - Re: [PATCH] Revert "KVM: x86: enable -Werror"

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: <87v9ns1z79.fsf@mpe.ellerman.id.au>
Date:   Thu, 27 Feb 2020 14:33:30 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Christoph Hellwig <hch@....de>, torvalds@...ux-foundation.org
Cc:     kvm@...r.kernel.org, pbonzini@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "KVM: x86: enable -Werror"

Christoph Hellwig <hch@....de> writes:
> This reverts commit ead68df94d248c80fdbae220ae5425eb5af2e753.
>
> Using the brain dead Werror flag fixes the build for me due to mostly
> harmless KASAN or similar warnings:
>
> arch/x86/kvm/x86.c: In function ‘kvm_timer_init’:
> arch/x86/kvm/x86.c:7209:1: error: the frame size of 1112 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>  7209 | }
>
> Feel free to add a CONFIG_WERROR if you care strong enough, but don't break
> peoples builds for absolutely no good reason.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>  arch/x86/kvm/Makefile | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 4654e97a05cc..b19ef421084d 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  ccflags-y += -Iarch/x86/kvm
> -ccflags-y += -Werror
>  
>  KVM := ../../../virt/kvm

We've had -Werror enabled by default on powerpc for over 10 years, and
haven't had many complaints. Possibly that's just because no one builds
powerpc kernels ...

The key thing is that it's configurable, so if it does break the build
for someone they can just turn it off. It's also off by default for
allyes/allmod builds because the user-selectable option disables
-Werror, eg:

config PPC_DISABLE_WERROR
	bool "Don't build arch/powerpc code with -Werror"
	help
	  This option tells the compiler NOT to build the code under
	  arch/powerpc with the -Werror flag (which means warnings
	  are treated as errors).

	  Only enable this if you are hitting a build failure in the
	  arch/powerpc code caused by a warning, and you don't feel
	  inclined to fix it.

config PPC_WERROR
	bool
	depends on !PPC_DISABLE_WERROR
	default y


In fact these days with shell support in Kconfig we could make it even
smarter, eg:

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 0b063830eea8..e7564f7cabc6 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -2,6 +2,7 @@
 
 config PPC_DISABLE_WERROR
        bool "Don't build arch/powerpc code with -Werror"
+       default $(success,whoami | grep hch)
        help
          This option tells the compiler NOT to build the code under
          arch/powerpc with the -Werror flag (which means warnings


cheers