v2-0001-Fix-bug-in-read_stream.c-s-split-IO-handling.patch

From 619aea104351cf1bc103fa7161e62e93bbacafa4 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Tue, 5 Aug 2025 14:35:09 +1200
Subject: [PATCH v2] Fix bug in read_stream.c's split IO handling.

If a circular queue wraparound, a multi-block IO split and a transition
to the fast path happened in a certain sequence, a buffer forwarded from
one StartReadBuffers() call to next would not be cleared out.  That
could confuse a later queue-wrapping StartReadBuffers() call by passing
it a random buffer.

Fix, and add some tighter and earlier assertions about the layout and
identity of buffers forwarded across IO splits, and the following
entries that should have been cleared before reuse.

Defect in commit ed0b87ca.

Bug: 19006
Backpatch-through: 18
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/19006-80fcaaf69000377e%40postgresql.org
---
 src/backend/storage/aio/read_stream.c | 34 +++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c
index 0e7f5557f5c..031fde9f4cb 100644
--- a/src/backend/storage/aio/read_stream.c
+++ b/src/backend/storage/aio/read_stream.c
@@ -247,12 +247,33 @@ read_stream_start_pending_read(ReadStream *stream)
 	Assert(stream->pinned_buffers + stream->pending_read_nblocks <=
 		   stream->max_pinned_buffers);
 
+#ifdef USE_ASSERT_CHECKING
 	/* We had better not be overwriting an existing pinned buffer. */
 	if (stream->pinned_buffers > 0)
 		Assert(stream->next_buffer_index != stream->oldest_buffer_index);
 	else
 		Assert(stream->next_buffer_index == stream->oldest_buffer_index);
 
+	/*
+	 * Pinned buffers forwarded by a preceding StartReadBuffers() call that
+	 * had to split the operation should match the leading blocks of this
+	 * following StartReadBuffers() call.
+	 */
+	Assert(stream->forwarded_buffers <= stream->pending_read_nblocks);
+	for (int i = 0; i < stream->forwarded_buffers; ++i)
+		Assert(BufferGetBlockNumber(stream->buffers[stream->next_buffer_index + i]) ==
+			   stream->pending_read_blocknum + i);
+
+	/*
+	 * Check that we've cleared the queue/overflow entries corresponding to
+	 * the rest of the blocks covered by this read, unless it's the first go
+	 * around and we haven't even initialized them yet.
+	 */
+	for (int i = stream->forwarded_buffers; i < stream->pending_read_nblocks; ++i)
+		Assert(stream->next_buffer_index + i >= stream->initialized_buffers ||
+			   stream->buffers[stream->next_buffer_index + i] == InvalidBuffer);
+#endif
+
 	/* Do we need to issue read-ahead advice? */
 	flags = stream->read_buffers_flags;
 	if (stream->advice_enabled)
@@ -979,6 +1000,19 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data)
 		stream->pending_read_nblocks == 0 &&
 		stream->per_buffer_data_size == 0)
 	{
+		/*
+		 * The fast path spins on one buffer entry repeatedly instead of
+		 * rotating through the whole queue and clearing the entries behind
+		 * it.  If the buffer it starts with happened to be forwarded between
+		 * StartReadBuffers() calls and also wrapped around the circular queue
+		 * partway through, then a copy also exists in the overflow zone, and
+		 * it won't clear it out as the regular path would.  Do that now, so
+		 * it doesn't need code for that.
+		 */
+		if (stream->oldest_buffer_index < stream->io_combine_limit - 1)
+			stream->buffers[stream->queue_size + stream->oldest_buffer_index] =
+				InvalidBuffer;
+
 		stream->fast_path = true;
 	}
 #endif
-- 
2.39.5 (Apple Git-154)