On Tue, 29 Aug 2023 at 19:13, Linus Torvalds
<[email protected]> wrote:
>
> On Mon, 28 Aug 2023 at 20:44, Kees Cook <[email protected]> wrote:
> >
> > On Mon, Aug 28, 2023 at 06:44:02PM -0700, Linus Torvalds wrote:
> > > The only thing that is new is the kernel pstore implementation. Why
> > > was this not a problem before? The warning existed back then too, but
> > > I never actually got it.
> >
> > Right -- if the compression method from before was different, it'll fail
> > now. (i.e. we removed everything but zlib.)
>
> I don't think that was the case.
>
> Looking back in my logs, I see lines like this:
>
> Aug 07 16:59:29 ryzen kernel: pstore: Using crash dump compression: deflate
>
> and while it appears F37 used to support other formats, it does have
>
> CONFIG_PSTORE_DEFLATE_COMPRESS_DEFAULT=y
>
> so it should all be zlib-compatible from what I can tell.
>
-5 == Z_BUF_ERROR which is only returned by zlib_inflate() in one
particular case (according the the kerneldoc):
In this implementation, the flush parameter of inflate() only affects the
return code (per zlib.h). inflate() always writes as much as possible to
strm->next_out, given the space available and the provided input--the effect
documented in zlib.h of Z_SYNC_FLUSH. Furthermore, inflate() always defers
the allocation of and copying into a sliding window until necessary, which
provides the effect documented in zlib.h for Z_FINISH when the entire input
stream available. So the only thing the flush parameter actually does is:
when flush is set to Z_FINISH, inflate() cannot return Z_OK. Instead it
will return Z_BUF_ERROR if it has not reached the end of the stream.
and the crypto compress wrapper for inflate does
ret = zlib_inflate(stream, Z_SYNC_FLUSH);
/*
* Work around a bug in zlib, which sometimes wants to taste an extra
* byte when being used in the (undocumented) raw deflate mode.
* (From USAGI).
*/
if (ret == Z_OK && !stream->avail_in && stream->avail_out) {
u8 zerostuff = 0;
stream->next_in = &zerostuff;
stream->avail_in = 1;
ret = zlib_inflate(stream, Z_FINISH);
}
IOW, it does not use Z_FINISH but Z_SYNC_FLUSH for the primary
invocation, and only stuffs in one additional NUL byte if it returns
Z_OK instead of Z_STREAM_END.
This is an oversight on my part. The diff below plugs this into the pstore code
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -593,7 +593,13 @@ static void decompress_record(struct pstore_record *record,
zstream->next_out = workspace;
zstream->avail_out = psinfo->bufsize;
- ret = zlib_inflate(zstream, Z_FINISH);
+ ret = zlib_inflate(zstream, Z_SYNC_FLUSH);
+ if (ret == Z_OK && !zstream->avail_in && zstream->avail_out) {
+ u8 zerostuff = 0;
+ zstream->next_in = &zerostuff;
+ zstream->avail_in = 1;
+ ret = zlib_inflate(zstream, Z_FINISH);
+ }
if (ret != Z_STREAM_END) {
pr_err("zlib_inflate() failed, ret = %d!\n", ret);
kvfree(workspace);