From 2f0a19b7e64ffab80f35380dbeb0e53696a52ac8 Mon Sep 17 00:00:00 2001 From: Zhong ShiHao Date: Fri, 4 Sep 2026 23:18:12 -0400 Subject: [PATCH v2] pg_surgery: bound the page's max offset by MaxHeapTuplesPerPage A corrupt pd_lower can push PageGetMaxOffsetNumber() past MaxHeapTuplesPerPage and overrun the include_this_tid[] stack array in heap_force_kill()/heap_force_freeze(). Skip such a block. heap_force_freeze() also dereferences the tuple a line pointer points at, which need not lie within the page, so skip a freeze whose line pointer is not a MAXALIGNed tuple between pd_upper and pd_special. --- contrib/pg_surgery/Makefile | 1 + contrib/pg_surgery/heap_surgery.c | 35 ++++++++++ contrib/pg_surgery/meson.build | 5 ++ contrib/pg_surgery/t/001_corrupt_page.pl | 87 ++++++++++++++++++++++++ 4 files changed, 128 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..b622437ce67 100644 --- a/contrib/pg_surgery/heap_surgery.c +++ b/contrib/pg_surgery/heap_surgery.c @@ -184,6 +184,19 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt) maxoffset = PageGetMaxOffsetNumber(page); + if (maxoffset > MaxHeapTuplesPerPage) + { + UnlockReleaseBuffer(buf); + + /* Update the current_start_ptr before moving to the next page. */ + curr_start_ptr = next_start_ptr; + + ereport(NOTICE, + (errmsg("skipping block %u for relation \"%s\" because the page header is invalid", + blkno, RelationGetRelationName(rel)))); + continue; + } + /* * Figure out which TIDs we are going to process and which ones we are * going to skip. @@ -229,6 +242,28 @@ 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) + { + PageHeader phdr = (PageHeader) page; + + if (ItemIdGetLength(itemid) < SizeofHeapTupleHeader || + ItemIdGetOffset(itemid) != MAXALIGN(ItemIdGetOffset(itemid)) || + ItemIdGetOffset(itemid) < phdr->pd_upper || + ItemIdGetOffset(itemid) + ItemIdGetLength(itemid) > phdr->pd_special) + { + ereport(NOTICE, + (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..ba9318d7d3e --- /dev/null +++ b/contrib/pg_surgery/t/001_corrupt_page.pl @@ -0,0 +1,87 @@ +# 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'); + +# 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 = BLCKSZ so the page's max +# offset (derived from pd_lower) is far above MaxHeapTuplesPerPage. +overwrite($hdr_path, 12, pack('S*', 8192, 8192, 8192)); + +# t_lp: leave the header alone, but point the first line pointer past the +# end of the page. Item is (lp_off = 32767, lp_flags = LP_NORMAL, +# lp_len = 100) packed into the 32-bit ItemIdData word. +overwrite($lp_path, 24, pack('L', 32767 | (1 << 15) | (100 << 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'); + +# 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: $!"); +} -- 2.37.1 (Apple Git-137.1)