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

On Mon, Mar 22, 2021 at 5:22 AM Tom Lane <tgl@sss.pgh.pa.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