From 9eec297b177dc3d187d83eb7add03d6367081a1d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 18 Aug 2026 15:00:15 -0400
Subject: [PATCH v1 1/3] GiST: Invalidate killed items consistently.

GiST neglected to invalidate its killedItems[] array on a rescan.  As a
result, it was just about possible for the wrong tuples from the wrong
index page to be LP_DEAD-marked on a rescan.  The scan mistakenly
believed that the previous rescan's killedItems[] were for this rescan's
curBlkno, causing index corruption (corruption has only been proven on
the master branch; see explanation below).

To fix, bring GiST in line with nbtree and hash: call gistkillitems()
from both gistrescan() and gistendscan() (in addition to the existing
gistgettuple caller).  That way the scan's pending killedItems[] are
passed to gistkillitems while they still describe items from curBlkno.
When gistkillitems runs, it'll invalidate the array in passing (and
won't needlessly miss out on an opportunity to LP_DEAD-mark eligible
index tuples).

This only became a serious problem in commit 9c9ddf109, which isn't on
any stable branch.  While we were just as sloppy about invalidating
killedItems[] state before that commit, it probably didn't cause any
real harm.  The test case that actually proved index corruption on the
master branch couldn't do so on any stable branch.  The test would
clobber curPageLSN (without also invalidating killedItems[] or
curBlkno), which accidentally prevented corruption.  It's far from
obvious that that's reliable, which is why this is being treated as a
bug affecting all stable branches.

Back branches get minimal hardening that just invalidates killedItems[]
in places where the master branch now calls gistkillitems.

Reviewed-By: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAH2-WzmwEThnQf17Ju+t0N9_KJLsEQSXzYrFnaS2=s4KnGGrqw@mail.gmail.com
Backpatch-through: 14
---
 src/include/access/gist_private.h  |  1 +
 src/backend/access/gist/gistget.c  | 40 ++++++++++++++++--------------
 src/backend/access/gist/gistscan.c | 10 ++++++++
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index 44514f1cb..39fdd27f5 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -458,6 +458,7 @@ extern XLogRecPtr gistXLogSplit(bool page_is_leaf,
 								Buffer leftchildbuf, bool markfollowright);
 
 /* gistget.c */
+extern void gistkillitems(IndexScanDesc scan);
 extern bool gistgettuple(IndexScanDesc scan, ScanDirection dir);
 extern int64 gistgetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
 extern bool gistcanreturn(Relation index, int attno);
diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index 5df2d4022..613e6d46d 100644
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -35,25 +35,27 @@
  * flag any entries because it is possible that the old entry was vacuumed
  * away and the TID was re-used by a completely different heap tuple.
  */
-static void
+void
 gistkillitems(IndexScanDesc scan)
 {
 	GISTScanOpaque so = (GISTScanOpaque) scan->opaque;
+	int			numKilled = so->numKilled;
 	Buffer		buffer;
 	Page		page;
-	OffsetNumber offnum;
-	ItemId		iid;
-	int			i;
 	bool		killedsomething = false;
 
 	Assert(so->curBlkno != InvalidBlockNumber);
 	Assert(XLogRecPtrIsValid(so->curPageLSN));
 	Assert(so->killedItems != NULL);
+	Assert(numKilled > 0);
+
+	/*
+	 * Always reset the scan state, so we don't look for same items on other
+	 * pages
+	 */
+	so->numKilled = 0;
 
 	buffer = ReadBuffer(scan->indexRelation, so->curBlkno);
-	if (!BufferIsValid(buffer))
-		return;
-
 	LockBuffer(buffer, GIST_SHARE);
 	gistcheckpage(scan->indexRelation, buffer);
 	page = BufferGetPage(buffer);
@@ -64,7 +66,10 @@ gistkillitems(IndexScanDesc scan)
 	 * safe.
 	 */
 	if (BufferGetLSNAtomic(buffer) != so->curPageLSN)
-		goto unlock;
+	{
+		UnlockReleaseBuffer(buffer);
+		return;
+	}
 
 	Assert(GistPageIsLeaf(page));
 
@@ -72,8 +77,11 @@ gistkillitems(IndexScanDesc scan)
 	 * Mark all killedItems as dead. We need no additional recheck, because,
 	 * if page was modified, curPageLSN must have changed.
 	 */
-	for (i = 0; i < so->numKilled; i++)
+	for (int i = 0; i < numKilled; i++)
 	{
+		OffsetNumber offnum = so->killedItems[i];
+		ItemId		iid = PageGetItemId(page, offnum);
+
 		if (!killedsomething)
 		{
 			/*
@@ -82,11 +90,12 @@ gistkillitems(IndexScanDesc scan)
 			 * there's no point continuing.
 			 */
 			if (!BufferBeginSetHintBits(buffer))
-				goto unlock;
+			{
+				UnlockReleaseBuffer(buffer);
+				return;
+			}
 		}
 
-		offnum = so->killedItems[i];
-		iid = PageGetItemId(page, offnum);
 		ItemIdMarkDead(iid);
 		killedsomething = true;
 	}
@@ -97,14 +106,7 @@ gistkillitems(IndexScanDesc scan)
 		BufferFinishSetHintBits(buffer, true, true);
 	}
 
-unlock:
 	UnlockReleaseBuffer(buffer);
-
-	/*
-	 * Always reset the scan state, so we don't look for same items on other
-	 * pages.
-	 */
-	so->numKilled = 0;
 }
 
 /*
diff --git a/src/backend/access/gist/gistscan.c b/src/backend/access/gist/gistscan.c
index 5583a5a35..b67ba5a8a 100644
--- a/src/backend/access/gist/gistscan.c
+++ b/src/backend/access/gist/gistscan.c
@@ -133,7 +133,13 @@ gistrescan(IndexScanDesc scan, ScanKey key, int nkeys,
 	int			i;
 	MemoryContext oldCxt;
 
+	/* Before leaving current page, deal with any killed items */
+	if (so->numKilled > 0)
+		gistkillitems(scan);
+
 	/* rescan an existing indexscan --- reset state */
+	so->curBlkno = InvalidBlockNumber;
+	so->curPageLSN = InvalidXLogRecPtr;
 
 	/*
 	 * The first time through, we create the search queue in the scanCxt.
@@ -349,6 +355,10 @@ gistendscan(IndexScanDesc scan)
 {
 	GISTScanOpaque so = (GISTScanOpaque) scan->opaque;
 
+	/* Before leaving current page, deal with any killed items */
+	if (so->numKilled > 0)
+		gistkillitems(scan);
+
 	/*
 	 * freeGISTstate is enough to clean up everything made by gistbeginscan,
 	 * as well as the queueCxt if there is a separate context for it.
-- 
2.53.0

