fix: avoid using uninitialized memory

[buriedpot]

Fix crash in RUM index scan when VACUUM removes all items from a posting tree leaf page

During a RUM index scan, if a concurrent VACUUM operation completes and removes all entries from a posting tree leaf page, the scan entry’s nlist field may become zero. In this state, the curItem pointer can end up referencing uninitialized or invalid memory, leading to a backend crash.

This commit resolves the issue by ensuring that curItem is properly handled (or not dereferenced) when nlist == 0, thus preventing undefined behavior during index scans under concurrent vacuuming.

Reproduction Steps

1. Prepare test data

CREATE EXTENSION rum;
ALTER SYSTEM SET enable_seqscan = off;
ALTER SYSTEM SET enable_indexscan = off;
ALTER SYSTEM SET enable_bitmapscan = on;
SELECT pg_reload_conf();

DROP TABLE IF EXISTS t;
CREATE TABLE t (id int, body tsvector);
ALTER TABLE t SET (autovacuum_enabled = FALSE);

INSERT INTO t
SELECT i, to_tsvector('rare word')
FROM generate_series(1, 100000) AS i;

CREATE INDEX ON t USING rum (body rum_tsvector_ops);

DELETE FROM t;  -- Leaves index entries pointing to dead tuples

2. Trigger the race condition using GDB

• Backend 1 (VACUUM):
  Attach GDB and set a breakpoint at rumVacuumPostingTreeLeaves.
  Run:

  VACUUM t;

  Let it pause at the breakpoint.

• Backend 2 (Query):
  Attach another GDB instance and set a breakpoint at startScanEntry.
  Run:

  SELECT * FROM t WHERE body @@ 'rare'::tsquery;

  Let it pause at the breakpoint.

• Resume execution:
  First, continue Backend 1 to completion (so VACUUM fully removes all items from the leaf page).
  Then, continue Backend 2.

  → The query backend will crash due to accessing invalid memory via entry->curItem when entry->nlist == 0.

This fix ensures safe handling of empty posting lists during scans, eliminating the crash under concurrent VACUUM and query workloads. The related issue is #168 and #99.

[buriedpot]

I strongly recommend prioritizing the review and applying of this fix.

[buriedpot]

You can observe that a concurrent VACUUM may set a page’s maxoff to zero in rumvacuum.c:

			if (newMaxOff > 0)
				memcpy(RumDataPageGetData(newPage), cleaned, newSize);

			pfree(cleaned);
			RumPageGetOpaque(newPage)->maxoff = newMaxOff;
			updateItemIndexes(newPage, attnum, &gvs->rumstate);

When newMaxOff == 0, the page ends up with no valid items.
Later, during a RUM index scan, the query backend reads this maxoff as entry->nlist. If nlist == 0, the subsequent logic in rumget.c still attempts to initialize entry->curItem:

			entry->nlist = maxoff;

			ptr = RumDataPageGetData(page);

			for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i))
			{
				ptr = rumDataPageLeafRead(ptr, entry->attnum, &item, true,
										  rumstate);
				entry->list[i - FirstOffsetNumber] = item;
			}

			LockBuffer(entry->buffer, RUM_UNLOCK);
			entry->isFinished = setListPositionScanEntry(rumstate, entry);
			if (!entry->isFinished)
				entry->curItem = entry->list[entry->offset];

Since the loop body is never executed when maxoff == 0, entry->list remains uninitialized. Yet, if setListPositionScanEntry() returns false (which can happen even with an empty list under certain scan states), entry->curItem will point to garbage memory — leading to a crash.

[arseny114]

Hi! Thanks for the report.
Sorry I didn't respond sooner; I was trying to reproduce the problem before commenting. Unfortunately, the PR was closed in the meantime.

For some reason, I couldn't reproduce it the first time, but after a while I tried again and the problem was reproduced.

Could you please reopen the pull request? I'll review and merge it soon.

[buriedpot]

Hi! Thanks for the report. Sorry I didn't respond sooner; I was trying to reproduce the problem before commenting. Unfortunately, the PR was closed in the meantime.

For some reason, I couldn't reproduce it the first time, but after a while I tried again and the problem was reproduced.

Could you please reopen the pull request? I'll review and merge it soon.

OK. I have a comprehensive fix that has been verified. Please allow me to update this PR—just a moment.

[buriedpot]

Hi! Thanks for the report. Sorry I didn't respond sooner; I was trying to reproduce the problem before commenting. Unfortunately, the PR was closed in the meantime.

For some reason, I couldn't reproduce it the first time, but after a while I tried again and the problem was reproduced.

Could you please reopen the pull request? I'll review and merge it soon.

Hi, I have updated the commit.

[arseny114]

Hi, I have updated the commit.

Thanks!

[samrose]

Is there anything we can do to help get this fix merged and released? Thank you!

[arseny114]

Is there anything we can do to help get this fix merged and released? Thank you!

Hi!
We are currently reviewing the fix and plan to complete it soon. Thanks for your patience!

[Strijdhagen]

Just a note that I'm having this issue in production atm. Apreciate all your hard work on the fix!

[arseny114]

@buriedpot, hi! Thanks again for the issue and the proposed fix.

I spent quite some time trying to fully understand the problem, since I couldn't reproduce the crash consistently. It turned out that the crash only happens reliably when palloc() returns zero-filled memory (which is confirmed by the backtrace you attached to the issue). In this case, the scan proceeds all the way to the consistentFn() call and crashes.

When the allocated memory happens to be filled with garbage, the scanner (in entryGetNextItem()) treats curItem.iptr as valid and walks right through the pages until it runs out of them, so no crash occurs. To reliably reproduce the issue, it's enough to replace palloc() with palloc0() in the problematic places and follow the rest of the steps. This will trigger the exact same stack trace as shown in the issue:

#0  0x000061a7e68cdd9a in pg_detoast_datum (datum=0x0) at fmgr.c:1834
#1  0x00007242b0fb874f in checkcondition_rum (checkval=0x7ffdbb454b40, val=0x61a7f9d63ab0, data=0x0) at src/rum_ts_utils.c:294
#2  0x00007242b0fb9391 in rum_TS_execute (curitem=0x61a7f9d63ab0, arg=0x7ffdbb454b40, flags=1, 
    chkcond=0x7242b0fb864e <checkcondition_rum>) at src/rum_ts_utils.c:791
#3  0x00007242b0fb975b in rum_tsquery_consistent (fcinfo=0x7ffdbb454bd0) at src/rum_ts_utils.c:912
#4  0x00007242b0fd6d1d in FunctionCall10Coll (flinfo=0x61a7f9cd1470, collation=100, arg1=107374078411512, arg2=1, arg3=107374078999208, 
    arg4=1, arg5=107374078411120, arg6=107374078411361, arg7=107374078411072, arg8=107374078411192, arg9=107374078411536, 
    arg10=107374078411560) at src/rumutil.c:1107
#5  0x00007242b0fc95ca in callConsistentFn (rumstate=0x61a7f9ccf7d8, key=0x61a7f9cd41d0) at src/rumget.c:170
#6  0x00007242b0fce5ae in scanGetItemFast (scan=0x61a7f9d78698, advancePast=0x7ffdbb454e30, item=0x7ffdbb454e30, recheck=0x7ffdbb454e1f)
    at src/rumget.c:2211
#7  0x00007242b0fcea43 in scanGetItem (scan=0x61a7f9d78698, advancePast=0x7ffdbb454e30, item=0x7ffdbb454e30, recheck=0x7ffdbb454e1f)
    at src/rumget.c:2327
#8  0x00007242b0fceb5d in rumgetbitmap (scan=0x61a7f9d78698, tbm=0x61a7f9d79508) at src/rumget.c:2369
...

---

I've reviewed your fix and it looks good overall, but I found another situation where we encounter the same kind of problem. Here's how to reproduce it:

1. Prepare the data:

ALTER SYSTEM SET enable_seqscan = off;
ALTER SYSTEM SET enable_indexscan = on;
ALTER SYSTEM SET enable_bitmapscan = off;
SELECT pg_reload_conf();

DROP TABLE IF EXISTS t;
CREATE TABLE t (id int, body tsvector);
ALTER TABLE t SET (autovacuum_enabled = FALSE);

INSERT INTO t
SELECT i, to_tsvector('rare word')
FROM generate_series(1, 100000) AS i;

CREATE INDEX ON t USING rum (body rum_tsvector_addon_ops, id) WITH (attach='id', to='body', order_by_attach='TRUE');

DELETE FROM t;

2. Set a breakpoint at rumScanBeginPostingTree() and execute a query like this (the key is that the scan must choose the BackwardScanDirection):

SELECT * FROM t WHERE body @@ 'rare'::tsquery ORDER BY id <=| 2;

Then, similar to your reproduction steps, pause the scanner in GDB at the moment it transitions from an internal posting tree page to a leaf page, when it has already released the pin and lock on the internal page but hasn't yet acquired them on the leaf page (this is the line here). After that, run VACUUM t; and resume the scan. This leads to a crash (because entry->offset starts taking negative values).

This happens because the scanPage() function (called from entryGetNextItem()) incorrectly handles empty pages when scanning in backward direction. I think it's enough to add a check like this at the beginning of the function:

if (RumPageGetOpaque(page)->maxoff < FirstOffsetNumber)
    return false;

This won't reproduce on the PR branch because it's missing commit f557ab1fc11238898332a732f9aca07be1671ae9, which fixes a different issue related to choosing the wrong scan direction. To reproduce it, you'll need to merge master into your branch.

I'll also leave a few other suggestions as inline comments on the code.