linux-kernel - Re: Heads up: gcc miscompiling initramfs zlib decompression code at -O3

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: <CAHk-=wi4Rgo7f17AwYduEPKr6SEVq-mxRJiZ5S5X0hQ9RWmkYA@mail.gmail.com>
Date:   Mon, 3 May 2021 10:41:45 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:     Arnd Bergmann <arnd@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Jann Horn <jannh@...gle.com>,
        lkml <linux-kernel@...r.kernel.org>,
        arcml <linux-snps-arc@...ts.infradead.org>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Subject: Re: Heads up: gcc miscompiling initramfs zlib decompression code at -O3

On Fri, Apr 30, 2021 at 1:46 PM Vineet Gupta <Vineet.Gupta1@...opsys.com> wrote:
>
> I've hit a mainline gcc 10.2 (also gcc 9.3) bug which triggers at -O3
> causing wrong codegen.

So it does seem to be a gcc bug or at least mis-feature where gcc gets
the aliasing decision wrong when vectorizing the code.

That said, the fact that gcc even tries to vectorize the code shows
how pointless it was for us to (years ago) try to make it use 16-bit
accesses in the first place.

So can you try this patch that basically reverts some of those
kernel-specific changes to zlib's inffast.c - and does so by just
upgrading it to a newer version from a more modern zlib (which in this
case still means "from 2017", but that's the most recent version there
is).

This is a fairly quick hack, and I really tried to keep it to just
inffast.c and inftrees.c with a few minimal fixups to inflate.c
itself.

Most of the changes are for naming (which seems to be at least partly
for C++ namespace reasons, ie "this" -> "here"), but there's some
cleanup wrt maximum table sizes etc too.

NOTE! I have not tested this patch in any other way than "it compiles
with allmodconfig". The diff looks reasonable to me, but that's all
I'm really going to say.

Does this avoid the gcc -O3 problem for you?

It would be lovely if somebody also took a look at some of the other
zlib code, like inflate.c itself. But some of that code has rather
subtle changes for things like s390 hardware support, and we have
thihngs like our fallthrough rules etc, so it gets a bit hairier.

Which is why I did just this fairly minimal part.

Note that the commit message has a "Not-yet-signed-off-by:" from me.
That's purely because I wanted it to be obvious that this needs a lot
more testing - I'm not comfy with this patch unless somebody takes it
upon themselves to actually test it under different loads (and
different architectures).

                  Linus

View attachment "0001-Update-zlib-inffast-code-to-more-modern-zlib.patch" of type "text/x-patch" (35111 bytes)