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

[Ard Biesheuvel]

On Tue, 29 Aug 2023 at 19:13, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 28 Aug 2023 at 20:44, Kees Cook <keescook@xxxxxxxxxxxx> 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);