Re: pg_dump: Fix dangling pointer in EndCompressorZstd()

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, kuznetsovam@altlinux.org, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, egori@altlinux.org, nickel@altlinux.org
Date: 2025-04-17T11:01:20Z
Lists: pgsql-hackers
> On 17 Apr 2025, at 01:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Michael Paquier <michael@paquier.xyz> writes:
>> On Wed, Apr 16, 2025 at 04:19:02PM +0200, Daniel Gustafsson wrote:
>>> 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.
> 
>> Indeed.  I was just looking at applying what Alexander has sent
>> because what EndCompressorZstd() not doing what the other methods do
>> makes no sense.  Perhaps you are already on it, Daniel?
> 
> I think the actual reason for the difference is that the methods that
> are taking care to zero the pointer do so because they test the
> pointer themselves.  For instance in EndCompressorGzip, the test is
> needed because perhaps no data was sent so the struct never got made.
> It incidentally offers protection against a double call of that
> function, but I don't think that was the intended reason.
> 
> I don't have any big objection to zeroing the pointer in
> EndCompressorZstd, but I think the claim that it's precisely
> analogous to the other EndCompressor methods is faulty,
> because it has no similar test.

Right, it has no similar test as the state in private_data is needed for both
read and write whereas gzip for example only need it for write (deflate).
Pushed as it improves code hygiene.

--
Daniel Gustafsson




Commits

  1. pg_dump: Set private_date pointer to NULL in callback