Re: [HACKERS] Custom compression methods (mac+lz4.h)

On Mon, Mar 22, 2021 at 5:22 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Actually, after reading that closer, the problem only affects the
> case where the compressed-data-length passed to the function is
> a lie. So it shouldn't be a problem for our usage.
>
> Also, after studying the documentation for LZ4_decompress_safe
> and LZ4_decompress_safe_partial, I realized that liblz4 is also
> counting on the *output* buffer size to not be a lie. So we
> cannot pass it a number larger than the chunk's true decompressed
> size. The attached patch resolves the issue I'm seeing.

Okay, the fix makes sense. In fact, IMHO, in general also this fix
looks like an optimization, I mean when slicelength >=
VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
even in the case of pglz. So shall we put this check directly in
toast_decompress_datum_slice instead of handling it at the lz4 level?

Like this.

diff --git a/src/backend/access/common/detoast.c
b/src/backend/access/common/detoast.c
index bed50e8..099ac15 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -506,6 +506,10 @@ toast_decompress_datum_slice(struct varlena
*attr, int32 slicelength)

Assert(VARATT_IS_COMPRESSED(attr));

+ /* liblz4 assumes that slicelength is not an overestimate */
+ if (slicelength >= VARRAWSIZE_4B_C(attr))
+ return toast_decompress_datum(attr);
+
/*
* Fetch the compression method id stored in the compression header and
* decompress the data slice using the appropriate
decompression routine.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com