linux-kernel - Re: [GIT PULL] pstore updates for v6.6-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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXHde34XNukpbCcbScetWKzv9m7nX2WCw8-zspPKc5g4zw@mail.gmail.com>
Date:   Tue, 29 Aug 2023 23:43:37 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Kees Cook <keescook@...omium.org>, Kees Cook <kees@...nel.org>,
        linux-kernel@...r.kernel.org, Enlin Mu <enlin.mu@...soc.com>,
        Eric Biggers <ebiggers@...gle.com>,
        "Guilherme G. Piccoli" <gpiccoli@...lia.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Yunlong Xing <yunlong.xing@...soc.com>,
        Yuxiao Zhang <yuxiaozhang@...gle.com>
Subject: Re: [GIT PULL] pstore updates for v6.6-rc1

On Tue, 29 Aug 2023 at 20:04, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Tue, 29 Aug 2023 at 10:29, Ard Biesheuvel <ardb@...nel.org> wrote:
> >
> > This is an oversight on my part. The diff below plugs this into the pstore code
>
> Hmm. My reaction is that you should also add the comment about the
> "Work around a bug in zlib" issue, because this code makes no sense
> otherwise.
>

Naturally. But pasting the comment into the email body seemed unnecessary.

> Of course, potentially even better would be to actually move this fix
> into our copy of zlib. Does the bug / misfeature still exist in
> upstream zlib?
>

I have no idea. You are the one sitting on the only [potential]
reproducer I am aware of, and there is nothing in the git (or prior)
history that sheds any light on this. Could you copy one of those EFI
variables to a file and share it so I can try and reproduce this?

> Also, grepping around a bit, I note that btrfs, for example, just does
> that Z_SYNC_FLUSH, and then checks for Z_OK. None of this Z_STREAM_END
> stuff.
>
> Similarly, going off to the debian code search, I find other code that
> just accepts either Z_OK or Z_STREAM_END.
>
> So what's so magical about how pstore uses zlib? This is just very odd.
>

AIUI, zlib can be used in raw mode and with a header/footer. Passing
the wbits parameter as a negative number (!) will switch into raw
mode.

The btrfs and jffs2 occurrences both default to positive wbits, and
switch to negative in a very specific case that involves zlib
internals that I don't understand. crypto/deflate.c implements both
modes, and pstore always used the raw one in all cases.

The workaround in crypto/deflate.c is documented as being specific to
the raw mode, which is why it makes sense to at least verify whether
the same workaround that pstore-deflate was using when doing raw zlib
through the crypto API is still needed now that it calls zlib
directly.