From 1431683a3df66187dac74ededaf613f3d4ecaea6 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Mon, 14 Sep 2026 13:01:10 -0400 Subject: [PATCH v5 3/3] Reject an invalid TID before heap_lock_tuple() reads it An invalid TID reaching heap_lock_tuple() is handed to ReadBuffer() as InvalidBlockNumber, which is P_NEW, so the relation is extended by a block before the lock attempt fails. The uninitialized block is left behind and later breaks sequential scans with "invalid page in block". A caller that gets this far with such a TID has a bug, so fail cleanly instead. An earlier version of this check sat in table_tuple_lock(), but that was both too weak and in the wrong place. ItemPointerIsValid() only tests for a non-NULL pointer and a nonzero offset, so a TID like (InvalidBlockNumber, 1) passed it and still reached ReadBuffer(). And the hazard being guarded against is specific to heap's use of P_NEW, not a documented table AM invariant, so the generic layer is not the right place to enforce it. Checking the block number in heapam_tuple_lock() covers every caller of the heap AM's lock callback and leaves other AMs to state their own rules. The moved-partitions marker also encodes InvalidBlockNumber, with offset MovedPartitionsOffsetNumber, and is a legitimate value that the retry loop in heapam_tuple_lock() reports on its own terms, so it is allowed through. Note the offset has to be tested before calling ItemPointerIndicatesMovedPartitions(), which reads it through the checked accessor and would assert on an offset of zero. In an assert-enabled build ItemPointerGetBlockNumber() inside heap_lock_tuple() already trips on this, so the new check mainly buys a clean error instead of relation extension in a production build. Suggested-by: Andres Freund Reported-by: Nikolay Samokhvalov Discussion: https://postgr.es/m/CAM6Zo8wZOLnCWRO_tuuXVX9J4N4JN6GsEnk8WJtT0%3D_0zy-1dw%40mail.gmail.com --- src/backend/access/heap/heapam_handler.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 6adb760b54f..7b3467ef65b 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -278,6 +278,27 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot, Assert(TTS_IS_BUFFERTUPLE(slot)); + /* + * Reject a TID that does not name a heap block before it reaches + * ReadBuffer() below, which would read InvalidBlockNumber as P_NEW and + * extend the relation, leaving an uninitialized block behind that later + * breaks sequential scans with "invalid page in block". A caller that + * gets here with such a TID has a bug, so fail cleanly instead. + * + * The moved-partitions marker also encodes InvalidBlockNumber, but it is + * a legitimate value that the retry loop below reports on its own terms, + * so let it through. Test the offset first, since + * ItemPointerIndicatesMovedPartitions() reads it through the checked + * accessor, which would assert on an offset of 0. + */ + if (unlikely(ItemPointerGetBlockNumberNoCheck(tid) == InvalidBlockNumber && + (ItemPointerGetOffsetNumberNoCheck(tid) == InvalidOffsetNumber || + !ItemPointerIndicatesMovedPartitions(tid)))) + elog(ERROR, "cannot lock tuple with invalid TID (%u,%u) in relation \"%s\"", + ItemPointerGetBlockNumberNoCheck(tid), + ItemPointerGetOffsetNumberNoCheck(tid), + RelationGetRelationName(relation)); + tuple_lock_retry: tuple->t_self = *tid; result = heap_lock_tuple(relation, tuple, cid, mode, wait_policy, -- 2.50.1