From 30e846fb3bd76fd39ada8825d9d362b79e8ac199 Mon Sep 17 00:00:00 2001 From: Zhong ShiHao Date: Mon, 7 Sep 2026 19:57:38 -0400 Subject: [PATCH v4] pg_surgery: skip blocks and line pointers that are already corrupt A corrupt pd_lower can push PageGetMaxOffsetNumber() past MaxHeapTuplesPerPage, which makes heap_force_kill() and heap_force_freeze() run off the end of include_this_tid[]. Skip such a block. heap_force_freeze() also dereferences the tuple that a line pointer points at, and that tuple need not lie inside the page. Skip a freeze whose line pointer is not a MAXALIGNed, header-sized item between pd_upper and pd_special. Add a TAP test that feeds pg_surgery both kinds of corrupt page. --- contrib/pg_surgery/Makefile | 1 + contrib/pg_surgery/heap_surgery.c | 42 ++++++++ contrib/pg_surgery/meson.build | 5 + contrib/pg_surgery/t/001_corrupt_page.pl | 117 +++++++++++++++++++++++ 4 files changed, 165 insertions(+) create mode 100644 contrib/pg_surgery/t/001_corrupt_page.pl diff --git a/contrib/pg_surgery/Makefile b/contrib/pg_surgery/Makefile index a66776c4c41..d7607ed4427 100644 --- a/contrib/pg_surgery/Makefile +++ b/contrib/pg_surgery/Makefile @@ -10,6 +10,7 @@ DATA = pg_surgery--1.0.sql PGFILEDESC = "pg_surgery - perform surgery on a damaged relation" REGRESS = heap_surgery +TAP_TESTS = 1 ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/contrib/pg_surgery/heap_surgery.c b/contrib/pg_surgery/heap_surgery.c index 51f3f3c49eb..2af13a97101 100644 --- a/contrib/pg_surgery/heap_surgery.c +++ b/contrib/pg_surgery/heap_surgery.c @@ -149,6 +149,7 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt) Buffer vmbuf = InvalidBuffer; bool unlock_vmbuf = false; Page page; + PageHeader phdr; BlockNumber blkno; OffsetNumber curoff; OffsetNumber maxoffset; @@ -181,9 +182,31 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt) LockBufferForCleanup(buf); page = BufferGetPage(buf); + phdr = (PageHeader) page; maxoffset = PageGetMaxOffsetNumber(page); + /* + * A corrupt pd_lower can make the line pointer array look longer than + * a heap page could ever have. Skip such a block; otherwise we would + * overrun include_this_tid[] below. + */ + if (maxoffset > MaxHeapTuplesPerPage) + { + UnlockReleaseBuffer(buf); + + /* Update the current_start_ptr before moving to the next page. */ + curr_start_ptr = next_start_ptr; + + ereport(NOTICE, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("skipping block %u for relation \"%s\" because the page header is invalid", + blkno, RelationGetRelationName(rel)), + errdetail("Maximum offset %u exceeds the maximum possible value %d.", + maxoffset, (int) MaxHeapTuplesPerPage))); + continue; + } + /* * Figure out which TIDs we are going to process and which ones we are * going to skip. @@ -229,6 +252,25 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt) continue; } + /* + * A freeze dereferences the tuple, so its line pointer must point + * at a MAXALIGNed location within the page's tuple area that is + * large enough to hold a heap tuple header. A kill only marks the + * line pointer dead, which is safe even for a bogus pointer. + */ + if (heap_force_opt == HEAP_FORCE_FREEZE && + (ItemIdGetLength(itemid) < SizeofHeapTupleHeader || + MAXALIGN(ItemIdGetOffset(itemid)) != ItemIdGetOffset(itemid) || + ItemIdGetOffset(itemid) < phdr->pd_upper || + ItemIdGetOffset(itemid) + ItemIdGetLength(itemid) > phdr->pd_special)) + { + ereport(NOTICE, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("skipping tid (%u, %u) for relation \"%s\" because its line pointer is invalid", + blkno, offno, RelationGetRelationName(rel)))); + continue; + } + /* Mark it for processing. */ Assert(offno <= MaxHeapTuplesPerPage); include_this_tid[offno - 1] = true; diff --git a/contrib/pg_surgery/meson.build b/contrib/pg_surgery/meson.build index 88e16dcc1b2..da7032c9d53 100644 --- a/contrib/pg_surgery/meson.build +++ b/contrib/pg_surgery/meson.build @@ -32,4 +32,9 @@ tests += { 'heap_surgery', ], }, + 'tap': { + 'tests': [ + 't/001_corrupt_page.pl', + ], + }, } diff --git a/contrib/pg_surgery/t/001_corrupt_page.pl b/contrib/pg_surgery/t/001_corrupt_page.pl new file mode 100644 index 00000000000..3822203c6ab --- /dev/null +++ b/contrib/pg_surgery/t/001_corrupt_page.pl @@ -0,0 +1,117 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Feed pg_surgery pages that are already corrupt and check that it skips +# them with a NOTICE instead of reading or writing out of bounds. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; + +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init(no_data_checksums => 1); +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->start; +$node->safe_psql('postgres', 'CREATE EXTENSION pg_surgery'); + +# The header corruption below is built from the server's block size, so it +# stays valid for PageIsVerified() at any BLCKSZ. +my $block_size = $node->safe_psql('postgres', 'SHOW block_size'); + +# One table whose page header we damage, one whose line pointer we damage. +$node->safe_psql( + 'postgres', q{ + CREATE TABLE t_hdr (a int); + INSERT INTO t_hdr SELECT generate_series(1, 5); + CREATE TABLE t_lp (a int); + INSERT INTO t_lp SELECT generate_series(1, 5); + CHECKPOINT; +}); + +my $hdr_path = $node->safe_psql('postgres', + q{SELECT pg_relation_filepath('t_hdr')}); +my $lp_path = $node->safe_psql('postgres', + q{SELECT pg_relation_filepath('t_lp')}); + +$node->stop; + +# t_hdr: force pd_lower = pd_upper = pd_special = block_size so the page's +# maximum offset, derived from pd_lower, is far above MaxHeapTuplesPerPage. +# This keeps the header valid for PageIsVerified() at any block size. +overwrite($hdr_path, 12, pack('S*', $block_size, $block_size, $block_size)); + +# t_lp: leave the header alone, but overwrite the first line pointer with a +# 32-bit ItemIdData word describing an item outside the page. Which field +# gets which value depends on the platform's bit-field ordering: lp_off and +# lp_len decode as either 32736 and 128, or 128 and 32736. Both values are +# MAXALIGNed and at least as long as a heap tuple header, so either way the +# line pointer is rejected by a page boundary check rather than by the +# length or the alignment check. +overwrite($lp_path, 24, pack('L', 32736 | (1 << 15) | (128 << 17))); + +$node->start; + +# A corrupt header must be skipped, not overrun include_this_tid[]. +my ($ret, $out, $err) = $node->psql('postgres', + q{SELECT heap_force_kill('t_hdr'::regclass, ARRAY['(0,1)']::tid[])}); +is($ret, 0, 'kill on corrupt-header page returns cleanly'); +like($err, qr/because the page header is invalid/, + 'corrupt-header block is skipped with a NOTICE'); + +# A line pointer outside the page must not be dereferenced by a freeze. +($ret, $out, $err) = $node->psql('postgres', + q{SELECT heap_force_freeze('t_lp'::regclass, ARRAY['(0,1)']::tid[])}); +is($ret, 0, 'freeze on out-of-page line pointer returns cleanly'); +like($err, qr/because its line pointer is invalid/, + 'out-of-page line pointer is skipped by freeze'); + +# The freeze above must not have touched the page. +$node->stop; +is(read_line_pointer($lp_path), 32736 | (1 << 15) | (128 << 17), + 'skipped line pointer is left alone'); +$node->start; + +# A kill does not dereference the tuple, so it still marks that line +# pointer dead rather than skipping it. +($ret, $out, $err) = $node->psql('postgres', + q{SELECT heap_force_kill('t_lp'::regclass, ARRAY['(0,1)']::tid[])}); +is($ret, 0, 'kill on out-of-page line pointer returns cleanly'); +unlike($err, qr/because its line pointer is invalid/, + 'kill still processes an out-of-page line pointer'); + +# The server survived every operation above. +is($node->safe_psql('postgres', 'SELECT 1'), 1, 'server is still up'); + +$node->stop; + +done_testing(); + +sub overwrite +{ + my ($relpath, $offset, $bytes) = @_; + my $path = $node->data_dir . '/' . $relpath; + + open(my $fh, '+<', $path) or BAIL_OUT("open $path failed: $!"); + binmode $fh; + sysseek($fh, $offset, 0) or BAIL_OUT("sysseek failed: $!"); + syswrite($fh, $bytes) or BAIL_OUT("syswrite failed: $!"); + close($fh) or BAIL_OUT("close failed: $!"); +} + +sub read_line_pointer +{ + my ($relpath) = @_; + my $path = $node->data_dir . '/' . $relpath; + my $buf; + + open(my $fh, '<', $path) or BAIL_OUT("open $path failed: $!"); + binmode $fh; + sysseek($fh, 24, 0) or BAIL_OUT("sysseek failed: $!"); + sysread($fh, $buf, 4) == 4 or BAIL_OUT("sysread failed: $!"); + close($fh) or BAIL_OUT("close failed: $!"); + + return unpack('L', $buf); +} -- 2.37.1 (Apple Git-137.1)