linux-kernel - Re: [PATCH v3 3/8] lib: add zstd support to decompress

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: <611A224B-1CB3-4283-9783-87C184C8983A@fb.com>
Date:   Thu, 26 Mar 2020 19:03:54 +0000
From:   Nick Terrell <terrelln@...com>
To:     Petr Malat <oss@...at.biz>
CC:     Nick Terrell <nickrterrell@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chris Mason <clm@...com>,
        "linux-kbuild@...r.kernel.org" <linux-kbuild@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>,
        Kernel Team <Kernel-team@...com>,
        Adam Borowski <kilobyte@...band.pl>,
        Patrick Williams <patrickw3@...com>,
        Michael van der Westhuizen <rmikey@...com>,
        "mingo@...nel.org" <mingo@...nel.org>,
        Patrick Williams <patrick@...cx.xyz>
Subject: Re: [PATCH v3 3/8] lib: add zstd support to decompress

> On Mar 26, 2020, at 9:47 AM, Petr Malat <oss@...at.biz> wrote:
> 
> Hi!
> On Wed, Mar 25, 2020 at 12:58:44PM -0700, Nick Terrell wrote:
>> From: Nick Terrell <terrelln@...com>
>> * Add unzstd() and the zstd decompress interface.
> Here I do not understand why you limit the window size to 8MB even when
> you read a larger value from the header. I do not see a reason why there
> should be such a limitation at the first place and if there should be,
> why it differs from ZSTD_WINDOWLOG_MAX.

When we are doing streaming decompression (either flush or fill is provided)
we have to allocate memory proportional to the window size. We want to
bound that memory so we don’t accidentally allocate too much memory.
When we are doing a single-pass decompression (neither flush nor fill
are provided) the window size doesn’t matter, and we only have to allocate
a fixed amount of memory ~192 KB.

The zstd spec [0] specifies that all decoders should allow window sizes
up to 8 MB. Additionally, the zstd CLI won’t produce window sizes greater
than 8 MB by default. The window size is controlled by the compression
level, and can be explicitly set.

I would expect larger window sizes to be beneficial for compression ratio,
though there is demising returns. I would expect that for kernel image
compression larger window sizes are beneficial, since it is decompressed
with a single pass. For initramfs decompression, I would expect that limiting
the window size could help decompression speed, since it uses streaming
compression, so unzstd() has to allocate a buffer of window size bytes.

> I removed that limitation to be able to test it in my environment and I
> found the performance is worst than with my patch by roughly 20% (on
> i7-3520M), which is a major drawback considering the main motivation
> to use zstd is the decompression speed. I will test on arm as well and
> share the result tomorrow.
>  Petr

What do you mean by that? Can you share with me the test you ran?
Is this for kernel decompression or initramfs decompression?

Best,
Nick

[0] https://tools.ietf.org/html/rfc8478#section-3.1.1.1.2