linux-hardening - Re: [GIT PULL] fallthrough fixes for Clang for 5.17-rc1

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] [day] [month] [year] [list]

Message-ID: <CAHk-=wjh5NjGvRm357n-v6wWU6J920hB4iG+nCtOHz2Qbs=p5A@mail.gmail.com>
Date:   Tue, 11 Jan 2022 17:02:05 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] fallthrough fixes for Clang for 5.17-rc1

On Tue, Jan 11, 2022 at 11:08 AM Gustavo A. R. Silva
<gustavoars@...nel.org> wrote:
>
> Please, pull the following patches that fix some fall-through warnings
> when building with Clang and -Wimplicit-fallthrough.

Ugh. I started pulling that, and then I actually looked at this one:

> Gustavo A. R. Silva (1):
>       MIPS: mm: tlbex: Fix fall-through warning for Clang

and that's just too ugly to live.

It was ugly before, but now it's just insane.

It literally has an if-statement with a block statement that contains
other cases.

Fine, that's not illegal, and Duff's device made the model famous.

But in this case, there's no actual _reason_ for it. It literally
looks like a mistake to me.

There's no reason to have that block statement in the first place, and
there's *doubly* no reason to add a "fallthrough" to other case
statements that then just do a "break" anyway.

I notice that we actually had that exact same pattern earlier, see
line 2166 in that same line. And it's equally bogus there. Actually,
it's even more bogus there, because the indentation is wrong too!

So I _think_ that patch should do something like the attached, but I
didn't actually test this in any way (I didn't check my mips
cross-build setup), and this is all so ugly that I have to avert my
eyes to even attach that patch to this email.

I also don't understand why it's doing that 'switch ()' on the
current_cpu_type(), only to avoid the 'cpu_has_mips_r2_exec_hazard'
test, Which is just _another_ switch on current_cpu_type().

All this code is completely incomprehensibly oddly written. It looks
like some kind of cut-and-paste thing with no actual understanding of
the code. And I refuse to make it worse than it already is by adding
even *more* mindless blather to it.

               Linus

View attachment "patch.diff" of type "text/x-patch" (1055 bytes)