mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix more portability issues in new amcheck code.

Browse Source 
verify_heapam() wasn't being careful to sanity-check tuple line
pointers before using them, resulting in SIGBUS on alignment-picky
architectures.  Fix that, add some more test coverage.

Mark Dilger, some tweaking by me

Discussion: https://postgr.es/m/30B8E99A-2D9C-48D4-A55C-741C9D5F1563@enterprisedb.com
This commit is contained in:
Tom Lane 2020-10-23 19:08:01 -04:00
parent 1b62d0fb3e
commit 321633e17b
2 changed files with 73 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
contrib/amcheck/t/001_verify_heapam.pl
View File

@ -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

78
contrib/amcheck/verify_heapam.c
View File

@ -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);