Messages in this thread | | | From | Ard Biesheuvel <> | Date | Tue, 29 Aug 2023 23:43:37 +0200 | Subject | Re: [GIT PULL] pstore updates for v6.6-rc1 |
| |
On Tue, 29 Aug 2023 at 20:04, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Tue, 29 Aug 2023 at 10:29, Ard Biesheuvel <ardb@kernel.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.
| |