public inbox for [email protected]  
help / color / mirror / Atom feed
From: JoongHyuk Shin <[email protected]>
To: [email protected]
Subject: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN()
Date: Fri, 17 Apr 2026 15:50:01 +0900
Message-ID: <CACSdjfMQYL3DV-3inrxdReqpFMOky4JxLbN0gTTY+qGJqrUJXw@mail.gmail.com> (raw)

Hi,

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.


Attachments:

  [application/octet-stream] 0001-Fix-TOCTOU-race-in-ReplicationSlotsComputeRequiredLS.patch (2.0K, 3-0001-Fix-TOCTOU-race-in-ReplicationSlotsComputeRequiredLS.patch)
  download | inline diff:
From 63c3b469d8e3addbe092861b06fb251afa4ac32a Mon Sep 17 00:00:00 2001
From: JoongHyuk Shin <[email protected]>
Date: Fri, 17 Apr 2026 15:07:11 +0900
Subject: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN()

ReplicationSlotsComputeRequiredLSN() released ReplicationSlotControlLock
before calling XLogSetReplicationSlotMinimumLSN(), creating a window
where a concurrent backend could compute a correct (lower) minimum LSN,
only for the first backend to overwrite it with a stale (higher) value.
This could lead to premature WAL removal.

The exact same pattern was already fixed for the xmin variant in commit
2a5225b99d7, which moved the lock release in
ReplicationSlotsComputeRequiredXmin() to after the global xmin update.
The LSN function was missed in that fix.

Move LWLockRelease() to after XLogSetReplicationSlotMinimumLSN() so the
lock is held for the entire compute-and-update sequence, matching the
xmin function's behavior.

Author: JoongHyuk Shin <[email protected]>
---
 src/backend/replication/slot.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 83fcde74718..0d87f0fd39d 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1312,6 +1312,11 @@ ReplicationSlotsComputeRequiredLSN(void)
 
 	Assert(ReplicationSlotCtl != NULL);
 
+	/*
+	 * Hold ReplicationSlotControlLock until after updating the minimum LSN.
+	 * Without this, a concurrent backend could compute a correct (lower)
+	 * minimum and then have it overwritten by our stale (higher) value.
+	 */
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 	for (i = 0; i < max_replication_slots + max_repack_replication_slots; i++)
 	{
@@ -1357,9 +1362,9 @@ ReplicationSlotsComputeRequiredLSN(void)
 			 restart_lsn < min_required))
 			min_required = restart_lsn;
 	}
-	LWLockRelease(ReplicationSlotControlLock);
 
 	XLogSetReplicationSlotMinimumLSN(min_required);
+	LWLockRelease(ReplicationSlotControlLock);
 }
 
 /*
-- 
2.52.0



view thread (3+ messages)  latest in thread

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]
  Subject: Re: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN()
  In-Reply-To: <CACSdjfMQYL3DV-3inrxdReqpFMOky4JxLbN0gTTY+qGJqrUJXw@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