linux-kernel - Re: [PATCH v2 2/2] linux/bits.h: Add compile time sanity check of GENMASK inputs

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:   Wed, 7 Aug 2019 17:58:54 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Joe Perches <joe@...ches.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Rikard Falkeborn <rikard.falkeborn@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Berg <johannes@...solutions.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] linux/bits.h: Add compile time sanity check of
 GENMASK inputs

On 8/7/19 5:07 PM, Joe Perches wrote:
> On Wed, 2019-08-07 at 23:55 +0900, Masahiro Yamada wrote:
>> On Wed, Aug 7, 2019 at 11:27 PM Guenter Roeck <linux@...ck-us.net> wrote:
> []
>>> Who is going to fix the fallout ? For example, arm64:defconfig no longer
>>> compiles with this patch applied.
>>>
>>> It seems to me that the benefit of catching misuses of GENMASK is much
>>> less than the fallout from no longer compiling kernels, since those
>>> kernels won't get any test coverage at all anymore.
>>
>> We cannot apply this until we fix all errors.
>> I do not understand why Andrew picked up this so soon.
> 
> I think it makes complete sense to break -next (not mainline)
> and force
> people to fix defects.  Especially these types of
> defects that are
> trivial to fix.
> 

I don't think this (from next-20190807):

Build results:
	total: 158 pass: 137 fail: 21
Qemu test results:
	total: 391 pass: 318 fail: 73

is very useful. The situation is bad enough for newly introduced problems.
It is all but impossible to get fixes for all problems discovered (or introduced)
by adding checks like this one. In some cases, no one will care. In others,
no one will pick up patches. Sometimes people won't know or realize that
they are expected to fix something. Making the situation worse, the failures
introduced by the new checks will hide other accumulating problems.

arch/sh has failed to build in mainline since 7/27 and in -next since
next-20190711, due to the added "fallthrough" warning. I don't think
that is too useful either. Ok, that situation may be a sign that the
architecture isn't maintained as well as it should, but I don't think
that this warrants breaking it on purpose in the hope to trigger
some kind of reaction.

I don't mind if new checks are introduced, and I agree that it is useful
and makes sense. But the checks should only be introduced after a reasonable
attempt was made to fix _all_ associated problems. That doesn't mean that
the entire work has to be done by the person introducing the check, but I
do see that person responsible for making sure (or a reasonable definition
of "make sure") that all problems are fixed before actually introducing
the check. Yes, I understand, this is a lot of work, but adding checks
and letting all hell break loose can not be the answer.

Guenter