Re: Use-after-free issue in postgres_fdw

On Tue, Mar 24, 2026 at 5:06 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Mar 19, 2026 at 11:56:13PM +0900, Etsuro Fujita wrote:
> > I got an offline report from my colleague Zhibai Song that
> > close_cursor() is called for a freed PGconn, leading to a server
> > crash. Here is a reproducer (the original reproducer he provided is a
> > bit complex, so I simplified it):
>
> Hmm. We have one isolation test with a pg_terminate_backend() that
> checks for a backend terminating, see temp-schema-cleanup.
>
> Do you think that it would be worth having a test for this scenario,
> where we could terminate a remote connection? An isolation test may
> work, I hope, providing some insurance with the order of the
> operations able to trigger the use-after-free behavior.

I added a simple SQL test, not an isolation one. Attached is an
updated patch for that. The basic idea for the test is the same as
the one I showed upthread, but the test ensures the order by checking
for the termination by using pg_stat_activity. In the patch I added a
comment for pgfdw_reject_incomplete_xact_state_change() as well.

> > I think the root cause is that it is too early to free the PGconn in
> > pgfdw_reject_incomplete_xact_state_change() even if the connection is
> > in a state where we cannot use it any further; I think we should delay
> > that until abort cleanup (ie, pgfdw_xact_callback()). Attached is a
> > patch for that.
>
> Removing the early disconnect() means that we should try to cover our
> tracks in all the existing ,code paths where we finish the
> top-transaction. Looking at the locations of
> pgfdw_reset_xact_state(), it looks like you are getting yourself
> covered.

Right, this patch delays discarding the failed connection until abort
cleanup of the top transaction, so it is guaranteed that anyone that
uses it can safely refer to its PGconn when needed.

Thanks for the comments! Sorry for the delay.

Best regards,
Etsuro Fujita