Re: [GIT PULL] pstore updates for v6.6-rc1

[Ard Biesheuvel]

On Tue, 29 Aug 2023 at 20:04, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 29 Aug 2023 at 10:29, Ard Biesheuvel <ardb@xxxxxxxxxx> 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.