public inbox for [email protected]  

help / color / mirror / Atom feed

From: JoongHyuk Shin <[email protected]>
To: Zhijie Hou (Fujitsu) <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN()
Date: Fri, 17 Apr 2026 16:46:31 +0900
Message-ID: <CACSdjfPtvQ9XQ5QLENHsofbPazqTT55Xufrq7XL_8QNEWtDJ1A@mail.gmail.com> (raw)
In-Reply-To: <TYRPR01MB14195CA431CFC783F77FB8AEF94202@TYRPR01MB14195.jpnprd01.prod.outlook.com>
References: <CACSdjfMQYL3DV-3inrxdReqpFMOky4JxLbN0gTTY+qGJqrUJXw@mail.gmail.com>
	<TYRPR01MB14195CA431CFC783F77FB8AEF94202@TYRPR01MB14195.jpnprd01.prod.outlook.com>

Hi Zhijie,

Thanks for pointing this out.
You're right that my patch only covers the read side
and misses the write side in ReplicationSlotReserveWal().
I should have checked for existing work before submitting.

I'll withdraw this patch.

Best Regards,
JoongHyuk Shin

On Fri, Apr 17, 2026 at 3:59 PM Zhijie Hou (Fujitsu) <[email protected]>
wrote:

> On Friday, April 17, 2026 2:50 PM JoongHyuk Shin <[email protected]>
> wrote:
> > Commit 2a5225b99d7 fixed a race in ReplicationSlotsComputeRequiredXmin()
> > where ReplicationSlotControlLock was released before the global xmin
> > update, allowing a concurrent backend to overwrite a correct value with
> > a stale one.
> >
> > ReplicationSlotsComputeRequiredLSN() has the same problem,
> > it releases the lock before calling XLogSetReplicationSlotMinimumLSN(),
> > so a stale minimum LSN can overwrite a correct (lower) one,
> > potentially leading to premature WAL removal.
> >
> > The attached patch moves LWLockRelease() to after the LSN update,
> > matching the xmin fix.
> > Since 2a5225b99d7 was backpatched to all supported versions,
> > I believe this should be as well.
>
> Thanks for noticing this. There is an existing thread [1] that I started
> following 2a5225b99d7 to address the same issue. The patch you posted
> only increases the lock scope in ReplicationSlotsComputeRequiredLSN() but
> does
> not increase the lock level when reserving WALs, so I think it would not
> fix the issue.
>
> Please feel free to review the patch in that thread if you find it helpful.
>
> [1] https://commitfest.postgresql.org/patch/6451/
>
> Best Regards,
> Hou zj
>

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN()
  In-Reply-To: <CACSdjfPtvQ9XQ5QLENHsofbPazqTT55Xufrq7XL_8QNEWtDJ1A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox