From c3645b0af895767c40b1ead2f2e076c7c33e482b Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Fri, 2 Apr 2021 15:07:52 -0700 Subject: [PATCH v18 4/4] amcheck: adding toast pointer corruption checks Verifying that toast pointer va_toastrelid fields match their heap table's reltoastrelid. Checking the extsize for a toast pointer against the raw size. This check could fail if buggy compression logic fails to notice that compressing the attribute makes it bigger. But assuming the logic for that is correct, overlarge extsize indicates a corrupted toast pointer. Checking the toast is not too large to be allocated. No such toasted value should ever be stored, but a corrupted toast pointer could record an unreasonbly large size, so check that. --- contrib/amcheck/verify_heapam.c | 34 +++++++++++++++++++- src/bin/pg_amcheck/t/004_verify_heapam.pl | 39 +++++++++++++++++++++-- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index cd1f2c4113..b2e121ed38 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1397,6 +1397,28 @@ check_tuple_attribute(HeapCheckContext *ctx) return true; } + /* The toast pointer had better point at the relation's toast table */ + if (toast_pointer.va_toastrelid != ctx->rel->rd_rel->reltoastrelid) + { + report_corruption(ctx, + psprintf("toast value %u toast relation oid %u differs from expected oid %u", + toast_pointer.va_valueid, + toast_pointer.va_toastrelid, + ctx->rel->rd_rel->reltoastrelid)); + return true; + } + + /* Compression should never expand the attribute */ + if (VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer) > toast_pointer.va_rawsize - VARHDRSZ) + { + report_corruption(ctx, + psprintf("toast value %u external size %u exceeds maximum expected for rawsize %u", + toast_pointer.va_valueid, + VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer), + toast_pointer.va_rawsize)); + return true; + } + /* If we were told to skip toast checking, then we're done. */ if (ctx->toast_rel == NULL) return true; @@ -1471,14 +1493,24 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) systable_endscan_ordered(toastscan); if (!found_toasttup) + { report_toast_corruption(ctx, ta, psprintf("toast value %u not found in toast table", ta->toast_pointer.va_valueid)); - else if (chunkno != (endchunk + 1)) + return; + } + + if (chunkno != (endchunk + 1)) report_toast_corruption(ctx, ta, psprintf("toast value %u was expected to end at chunk %u, but ended at chunk %u", ta->toast_pointer.va_valueid, (endchunk + 1), chunkno)); + + if (!AllocSizeIsValid(ta->toast_pointer.va_rawsize)) + report_toast_corruption(ctx, ta, + psprintf("toast value %u rawsize %u too large to be allocated", + ta->toast_pointer.va_valueid, + ta->toast_pointer.va_rawsize)); } /* diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl index 307f14611c..de525fbdd8 100644 --- a/src/bin/pg_amcheck/t/004_verify_heapam.pl +++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl @@ -224,7 +224,7 @@ my $rel = $node->safe_psql('postgres', qq(SELECT pg_relation_filepath('public.te my $relpath = "$pgdata/$rel"; # Insert data and freeze public.test -use constant ROWCOUNT => 16; +use constant ROWCOUNT => 19; $node->safe_psql('postgres', qq( INSERT INTO public.test (a, b, c) VALUES ( @@ -240,6 +240,13 @@ my $relfrozenxid = $node->safe_psql('postgres', my $datfrozenxid = $node->safe_psql('postgres', q(select datfrozenxid from pg_database where datname = 'postgres')); +# Find our toast relation id +my $toastrelid = $node->safe_psql('postgres', qq( + SELECT c.reltoastrelid + FROM pg_catalog.pg_class c + WHERE c.oid = 'public.test'::regclass + )); + # Sanity check that our 'test' table has a relfrozenxid newer than the # datfrozenxid for the database, and that the datfrozenxid is greater than the # first normal xid. We rely on these invariants in some of our tests. @@ -296,7 +303,7 @@ close($file) $node->start; # Ok, Xids and page layout look ok. We can run corruption tests. -plan tests => 19; +plan tests => 22; # Check that pg_amcheck runs against the uncorrupted table without error. $node->command_ok(['pg_amcheck', '-p', $port, 'postgres'], @@ -501,7 +508,7 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++) push @expected, qr/${header}multitransaction ID 4 equals or exceeds next valid multitransaction ID 1/; } - elsif ($offnum == 15) # Last offnum must equal ROWCOUNT + elsif ($offnum == 15) { # Set both HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI $tup->{t_infomask} |= HEAP_XMAX_COMMITTED; @@ -511,6 +518,32 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++) push @expected, qr/${header}multitransaction ID 4000000000 precedes relation minimum multitransaction ID threshold 1/; } + elsif ($offnum == 16) + { + # Corrupt column c's toast pointer va_toastrelid field + my $otherid = $toastrelid + 1; + $tup->{c_va_toastrelid} = $otherid; + $header = header(0, $offnum, 2); + push @expected, + qr/${header}toast value \d+ toast relation oid $otherid differs from expected oid $toastrelid/; + } + elsif ($offnum == 17) + { + # Corrupt column c's toast pointer va_extinfo field + $tup->{c_va_extinfo} = 7654321; + $header = header(0, $offnum, 2); + push @expected, + qr/${header}toast value \d+ external size 7654321 exceeds maximum expected for rawsize 10004/; + } + elsif ($offnum == 18) # Last offnum should equal ROWCOUNT-1 + { + # Corrupt column c's toast pointer va_rawsize field with a value + # exceeding maximum allowable allocation size + $tup->{c_va_rawsize} = 0x40000000; + $header = header(0, $offnum, 2); + push @expected, + qr/${header}toast value \d+ rawsize 1073741824 too large to be allocated/; + } write_tuple($file, $offset, $tup); } close($file) -- 2.21.1 (Apple Git-122.3)