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: <CAMj1kXGJCBj2JQFkUgd26H-abagcpO+7z_--6HV42VeaqCsEnQ@mail.gmail.com>
Date:   Tue, 29 Aug 2023 19:29:31 +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 19:13, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Mon, 28 Aug 2023 at 20:44, Kees Cook <keescook@...omium.org> 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);