From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Daniil Zakhlystov <usernamedt(at)yandex-team(dot)ru>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: zstd compression for pg_dump |
Date: | 2021-03-12 22:31:57 |
Message-ID: | 3866daa6-cc68-2dbe-e7db-0478f1c3d21a@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 1/4/21 11:17 AM, Daniil Zakhlystov wrote:
> Hi!
>
>> On Jan 4, 2021, at 11:04 AM, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>>
>> Daniil, is levels definition compatible with libpq compression patch?
>> +typedef struct Compress {
>> + CompressionAlgorithm alg;
>> + int level;
>> + /* Is a nondefault level set ? This is useful since different compression
>> + * methods have different "default" levels. For now we assume the levels
>> + * are all integer, though.
>> + */
>> + bool level_set;
>> +} Compress;
>
> Similarly to this patch, it is also possible to define the compression level at the initialization stage in libpq compression patch.
>
> The difference is that in libpq compression patch the default compression level always equal to 1, independently of the chosen compression algorithm.
>
>> On Jan 4, 2021, at 11:04 AM, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>>
>> Libpq compression encountered some problems with memory consumption which required some extra config efforts.
>
>
>> On Jan 4, 2021, at 12:06 PM, Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>>
>> RAM use is not significantly different from zlib, except that zstd --long adds
>> more memory.
>
> Regarding ZSTD memory usage:
>
> Recently I’ve made a couple of tests of libpq compression with different ZLIB/ZSTD compression levels which shown that compressing/decompressing ZSTD w/ high compression levels
> require to allocate more virtual (Commited_AS) memory, which may be exploited by malicious clients:
>
> https://www.postgresql.org/message-id/62527092-16BD-479F-B503-FA527AF3B0C2%40yandex-team.ru
>
> We can avoid high memory usage by limiting the max window size to 8MB. This should effectively disable the support of compression levels above 19:
> https://www.postgresql.org/message-id/6A45DFAA-1682-4EF2-B835-C5F46615EC49%40yandex-team.ru
>
> So maybe it is worthwhile to use similar restrictions in this patch.
>
I think there's a big difference between those two patches. In the libpq
case, the danger is that the client requests the server to compress the
data in a way that requires a lot of memory. I.e. the memory is consumed
on the server.
With this pg_dump patch, the compression is done by the pg_dump process,
not the server. So if the attacker configures the compression in a way
that requires a lot of memory, so what? He'll just allocate memory on
the client machine, where he could also just run a custom binary that
does a huge malloc().
So I don't think we need to worry about this too much.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2021-03-12 22:55:35 | Re: pg_amcheck contrib application |
Previous Message | Mark Dilger | 2021-03-12 22:24:18 | Re: pg_amcheck contrib application |