Re: pg_dump: Fix dangling pointer in EndCompressorZstd() - Mailing list pgsql-hackers

> On 16 Apr 2025, at 13:48, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>
> On Wed, Apr 16, 2025 at 1:57 PM Alexander Kuznetsov
> <kuznetsovam@altlinux.org> wrote:
>>
>> Hello everyone,
>>
>> We've found that EndCompressorZstd() doesn't set cs->private_data to NULL after pg_free(),
>> unlike other EndCompressor implementations.
>> While this doesn't currently cause issues (as the pointer soon gets reassigned),
>> we recommend fixing this to maintain consistency with other implementations and prevent potential future issues.
>>
>> The patch is attached, would appreciate your thoughts on this change.
>
> Thanks for the patch.
>
> The next thing that happens in EndCompressor() is freeing cs itself.
> So cs->private_data is not used anywhere, so no harm in the current
> code. But it's better to set to NULL since EndCompressorZstd()
> wouldn't know how it's being accessed after returning from there. The
> other implementation of CompressionState::end() EndCompressorGzip()
> calls DeflateCompressorEnd() which also sets cs->private_data
> explicitly. So should EndCompressorZstd().

Agreed, while it's perfectly safe today the end method should not make
assumptions about the use of the private_data pointer upon return and should
leave it set to NULL.

--
Daniel Gustafsson