diff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001_verify_heapam.pl index f8ee5384f3..1581e51f3c 100644 --- a/contrib/amcheck/t/001_verify_heapam.pl +++ b/contrib/amcheck/t/001_verify_heapam.pl @@ -4,7 +4,7 @@ use warnings; use PostgresNode; use TestLib; -use Test::More tests => 55; +use Test::More tests => 79; my ($node, $result); @@ -109,13 +109,17 @@ sub corrupt_first_page or BAIL_OUT("open failed: $!"); binmode $fh; - # Corrupt two line pointers. To be stable across platforms, we use - # 0x55555555 and 0xAAAAAAAA for the two, which are bitwise reverses of each - # other. + # Corrupt some line pointers. The values are chosen to hit the + # various line-pointer-corruption checks in verify_heapam.c + # on both little-endian and big-endian architectures. seek($fh, 32, 0) or BAIL_OUT("seek failed: $!"); - syswrite($fh, pack("L*", 0x55555555, 0xAAAAAAAA)) - or BAIL_OUT("syswrite failed: $!"); + syswrite( + $fh, + pack("L*", + 0xAAA15550, 0xAAA0D550, 0x00010000, + 0x00008000, 0x0000800F, 0x001e8000) + ) or BAIL_OUT("syswrite failed: $!"); close($fh) or BAIL_OUT("close failed: $!"); @@ -126,17 +130,23 @@ sub detects_heap_corruption { my ($function, $testname) = @_; - detects_corruption($function, $testname, - qr/line pointer redirection to item at offset \d+ exceeds maximum offset \d+/ + detects_corruption( + $function, + $testname, + qr/line pointer redirection to item at offset \d+ precedes minimum offset \d+/, + qr/line pointer redirection to item at offset \d+ exceeds maximum offset \d+/, + qr/line pointer to page offset \d+ is not maximally aligned/, + qr/line pointer length \d+ is less than the minimum tuple header size \d+/, + qr/line pointer to page offset \d+ with length \d+ ends beyond maximum page offset \d+/, ); } sub detects_corruption { - my ($function, $testname, $re) = @_; + my ($function, $testname, @re) = @_; my $result = $node->safe_psql('postgres', qq(SELECT * FROM $function)); - like($result, $re, $testname); + like($result, $_, $testname) for (@re); } sub detects_no_corruption diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 37b40a0404..8bb890438a 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -105,6 +105,7 @@ typedef struct HeapCheckContext OffsetNumber offnum; ItemId itemid; uint16 lp_len; + uint16 lp_off; HeapTupleHeader tuphdr; int natts; @@ -247,6 +248,13 @@ verify_heapam(PG_FUNCTION_ARGS) memset(&ctx, 0, sizeof(HeapCheckContext)); ctx.cached_xid = InvalidTransactionId; + /* + * If we report corruption when not examining some individual attribute, + * we need attnum to be reported as NULL. Set that up before any + * corruption reporting might happen. + */ + ctx.attnum = -1; + /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */ old_context = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory); random_access = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0; @@ -378,14 +386,22 @@ verify_heapam(PG_FUNCTION_ARGS) /* * If this line pointer has been redirected, check that it - * redirects to a valid offset within the line pointer array. + * redirects to a valid offset within the line pointer array */ if (ItemIdIsRedirected(ctx.itemid)) { OffsetNumber rdoffnum = ItemIdGetRedirect(ctx.itemid); ItemId rditem; - if (rdoffnum < FirstOffsetNumber || rdoffnum > maxoff) + if (rdoffnum < FirstOffsetNumber) + { + report_corruption(&ctx, + psprintf("line pointer redirection to item at offset %u precedes minimum offset %u", + (unsigned) rdoffnum, + (unsigned) FirstOffsetNumber)); + continue; + } + if (rdoffnum > maxoff) { report_corruption(&ctx, psprintf("line pointer redirection to item at offset %u exceeds maximum offset %u", @@ -401,8 +417,36 @@ verify_heapam(PG_FUNCTION_ARGS) continue; } - /* Set up context information about this next tuple */ + /* Sanity-check the line pointer's offset and length values */ ctx.lp_len = ItemIdGetLength(ctx.itemid); + ctx.lp_off = ItemIdGetOffset(ctx.itemid); + + if (ctx.lp_off != MAXALIGN(ctx.lp_off)) + { + report_corruption(&ctx, + psprintf("line pointer to page offset %u is not maximally aligned", + ctx.lp_off)); + continue; + } + if (ctx.lp_len < MAXALIGN(SizeofHeapTupleHeader)) + { + report_corruption(&ctx, + psprintf("line pointer length %u is less than the minimum tuple header size %u", + ctx.lp_len, + (unsigned) MAXALIGN(SizeofHeapTupleHeader))); + continue; + } + if (ctx.lp_off + ctx.lp_len > BLCKSZ) + { + report_corruption(&ctx, + psprintf("line pointer to page offset %u with length %u ends beyond maximum page offset %u", + ctx.lp_off, + ctx.lp_len, + (unsigned) BLCKSZ)); + continue; + } + + /* It should be safe to examine the tuple's header, at least */ ctx.tuphdr = (HeapTupleHeader) PageGetItem(ctx.page, ctx.itemid); ctx.natts = HeapTupleHeaderGetNatts(ctx.tuphdr); @@ -1088,25 +1132,6 @@ check_tuple(HeapCheckContext *ctx) bool fatal = false; uint16 infomask = ctx->tuphdr->t_infomask; - /* - * If we report corruption before iterating over individual attributes, we - * need attnum to be reported as NULL. Set that up before any corruption - * reporting might happen. - */ - ctx->attnum = -1; - - /* - * If the line pointer for this tuple does not reserve enough space for a - * complete tuple header, we dare not read the tuple header. - */ - if (ctx->lp_len < MAXALIGN(SizeofHeapTupleHeader)) - { - report_corruption(ctx, - psprintf("line pointer length %u is less than the minimum tuple header size %u", - ctx->lp_len, (uint32) MAXALIGN(SizeofHeapTupleHeader))); - return; - } - /* If xmin is normal, it should be within valid range */ xmin = HeapTupleHeaderGetXmin(ctx->tuphdr); switch (get_xid_status(xmin, ctx, NULL)) @@ -1256,6 +1281,9 @@ check_tuple(HeapCheckContext *ctx) for (ctx->attnum = 0; ctx->attnum < ctx->natts; ctx->attnum++) if (!check_tuple_attribute(ctx)) break; /* cannot continue */ + + /* revert attnum to -1 until we again examine individual attributes */ + ctx->attnum = -1; } /* @@ -1393,9 +1421,9 @@ get_xid_status(TransactionId xid, HeapCheckContext *ctx, if (!fxid_in_cached_range(fxid, ctx)) { /* - * We may have been checking against stale values. Update the - * cached range to be sure, and since we relied on the cached - * range when we performed the full xid conversion, reconvert. + * We may have been checking against stale values. Update the cached + * range to be sure, and since we relied on the cached range when we + * performed the full xid conversion, reconvert. */ update_cached_xid_range(ctx); fxid = FullTransactionIdFromXidAndCtx(xid, ctx);