linux-kernel - Re: [PATCH] kvm: x86: mmu: Add cast to negated bitmasks in update_permission

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]

Date:   Tue, 19 Jun 2018 16:45:43 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Joe Perches <joe@...ches.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>, rkrcmar@...hat.com,
        Thomas Gleixner <tglx@...utronix.de>, hpa@...or.com,
        x86@...nel.org, kvm@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>
Subject: Re: [PATCH] kvm: x86: mmu: Add cast to negated bitmasks in
 update_permission_bitmask()

On Tue, Jun 19, 2018 at 02:55:05PM -0700, Joe Perches wrote:
> On Tue, 2018-06-19 at 14:10 -0700, Matthias Kaehlcke wrote:
> > On Tue, Jun 19, 2018 at 12:11:27PM -0700, Joe Perches wrote:
> > > -Wconstant-conversion is only issued when a *constant value* is assigned to
> > an incompatible type.
> > 
> > > Trying to allow a "make W=3" to be compiler warning message free
> > > is also silly.
> > > 
> > > I think it's better to make the warning emitted only at a W=3
> > > level instead.
> > 
> > Another difference with -Wsign-conversion is that enabling it would
> > probably result in thousands of warnings. Do you have evidence that
> > there is a significant number of spurious -Wconstant-conversion
> > warnings?
> 
> Not my issue really.

Well, you advocate to disable a possibly useful warning globally ...

> You're advocating for making the code more complex/ugly for a
> condition where the result is identical.

My goal is no to make the code (slightly) more complex/ugly but have
the rest of the kernel benefit from a possibly useful warning.

> Do you have evidence this is the only location in
> an allyesconfig compilation?

I never claimed that this is the only location, but that it is not an
extremely noisy warning that can not be fixed without a major effort.

Since you asked:

v4.16 with an x86 allyesconfig and a few drivers disabled since they
don't build with clang (yet):

16 occurrences of -Wconstant-conversion, including the ones fixed by
this patch. Certainly no need for an endless churn of patches, and
given the limited number it also doesn't seem likely that new
instances will be added on a regular base.