From 2f41e74d211d0bb939bedd688933aa891c511b2d Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Thu, 15 Apr 2021 08:56:59 -0700 Subject: [PATCH v21] amcheck: adding toast pointer corruption checks Adding additional checks of toast pointers: checking the extsize against the rawsize, the uncompressed size against the size limit for varlena datums, the va_toastrelid field against the heap table's reltoastrelid, and if compressed, the validity of the compression method ID. Adding checks that the toasted attribute chunks are returned by the toast index scan in order and without duplicates, and improving the reports of missing or extra chunks to be more clear to the user. Changing the logic to continue checking toast even after reporting that HEAP_HASEXTERNAL is false. Previously, the toast checking stopped here, but that wasn't necessary, and subsequent checks may provide additional useful diagnostic information. --- contrib/amcheck/verify_heapam.c | 370 ++++++++++++++++++++++++++----- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 319 insertions(+), 52 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 9366f45d74..b5500c53a0 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -30,6 +30,9 @@ PG_FUNCTION_INFO_V1(verify_heapam); /* The number of columns in tuples returned by verify_heapam */ #define HEAPCHECK_RELATION_COLS 4 +/* The largest valid toast va_rawsize */ +#define VARLENA_SIZE_LIMIT 0x3FFFFFFF + /* * Despite the name, we use this for reporting problems with both XIDs and * MXIDs. @@ -146,12 +149,40 @@ typedef struct HeapCheckContext Tuplestorestate *tupstore; } HeapCheckContext; +/* + * Struct holding the running context information during the check of a + * a single toasted attribute. + */ +typedef struct ToastCheckContext +{ + /* + * Cache tracking a sequence of contiguous toast chunks, each of size + * TOAST_MAX_CHUNK_SIZE, and having sequence numbers outside the expected + * range. The sequence numbers of such chunks are cached until the + * sequence ends so that a single toast corruption report can be emitted + * for the group, rather than one report per chunk. + */ + bool have_extraneous_chunks; + int32 first_extraneous; + int32 last_extraneous; + + /* Most recent previously seen chunk sequence number */ + int32 last_chunk_seen; + + /* + * Expected sequence number and size of the final chunk expected for this + * toasted attribute + */ + int32 final_expected_chunk; + int32 final_expected_size; +} ToastCheckContext; + /* Internal implementation */ static void sanity_check_relation(Relation rel); static void check_tuple(HeapCheckContext *ctx); -static void check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, - ToastedAttribute *ta, int32 chunkno, - int32 endchunk); +static int32 check_toast_tuple(HeapTuple toasttup, ToastedAttribute *ta, + ToastCheckContext *tctx, HeapCheckContext *hctx, + int32 chunkno); static bool check_tuple_attribute(HeapCheckContext *ctx); static void check_toasted_attribute(HeapCheckContext *ctx, @@ -1147,10 +1178,152 @@ check_tuple_visibility(HeapCheckContext *ctx) return true; } +/* + * Issues toast corruption reports for the given extraneous partial chunk, if + * not null, along with any extraneous full chunks in the tctx cache, which is + * then cleared. A "partial chunk" is any chunk with size different from + * TOAST_MAX_CHUNK_SIZE. An "extraneous chunk" is one with a sequence number + * outside the expected range for the toasted attribute. + * + * To report extraneous full chunks and clear the cache, call with partialchunk + * and partialsize NULL. If the cache is already empty, the call is harmless. + * + * Extraneous partial chunks are never cached. When reporting one, any cached + * extraneous full chunks will also be reported and the cache cleared. + */ +static void +report_extraneous_chunks(HeapCheckContext *hctx, ToastedAttribute *ta, + ToastCheckContext *tctx, int32 *partialchunk, + int32 *partialsize) +{ + if (tctx->have_extraneous_chunks) + { + if (tctx->first_extraneous < tctx->last_extraneous) + report_toast_corruption(hctx, ta, + psprintf("toast value %u has unexpected chunks %d through %d each with size %d", + ta->toast_pointer.va_valueid, + tctx->first_extraneous, + tctx->last_extraneous, + (int)TOAST_MAX_CHUNK_SIZE)); + else + report_toast_corruption(hctx, ta, + psprintf("toast value %u has unexpected chunk %d with size %d", + ta->toast_pointer.va_valueid, + tctx->first_extraneous, + (int)TOAST_MAX_CHUNK_SIZE)); + tctx->have_extraneous_chunks = false; + } + + if (partialchunk != NULL) + report_toast_corruption(hctx, ta, + psprintf("toast value %u has unexpected chunk %d with size %d", + ta->toast_pointer.va_valueid, + *partialchunk, *partialsize)); +} /* - * Check the current toast tuple against the state tracked in ctx, recording - * any corruption found in ctx->tupstore. + * Records that a toast chunk should be reported as extraneous. After + * finishing all calls to this function for a given toasted attribute, a call + * to report_extraneous_chunks() should be issued to flush the cache. + */ +static void +handle_extraneous_chunk(HeapCheckContext *hctx, ToastedAttribute *ta, + ToastCheckContext *tctx, int32 curchunk, + int32 chunksize) +{ + if (chunksize == TOAST_MAX_CHUNK_SIZE) + { + if (tctx->have_extraneous_chunks) + { + if (tctx->last_extraneous == curchunk - 1) + { + /* + * This is the next chunk in an ongoing sequence. Extend it, + * but do not report it yet. + */ + tctx->last_extraneous = curchunk; + return; + } + + /* + * There is an ongoing sequence, but this chunk is discontiguous + * with it. Report the sequence and clear the cache so we can + * start over with this chunk. + */ + report_extraneous_chunks(hctx, ta, tctx, NULL, NULL); + } + + /* Start a new sequence, but do not report it yet. */ + tctx->first_extraneous = curchunk; + tctx->last_extraneous = curchunk; + tctx->have_extraneous_chunks = true; + return; + } + + /* + * This is a partial chunk. Report it. If there is an ongoing full chunk + * sequence, this will report and flush that, too, but we don't care. + */ + report_extraneous_chunks(hctx, ta, tctx, &curchunk, &chunksize); +} + +/* + * Issues toast corruption reports for one or more missing toast chunks + * in the [first_missing..last_missing] range, inclusive. + */ +static void +report_missing_chunks(HeapCheckContext *hctx, ToastedAttribute *ta, + ToastCheckContext *tctx, int32 first_missing, + int32 last_missing) +{ + int32 expected_size; + + Assert(first_missing >= 0); + Assert(last_missing >= first_missing); + Assert(last_missing <= tctx->final_expected_chunk); + + if (last_missing < tctx->final_expected_chunk) + expected_size = TOAST_MAX_CHUNK_SIZE; + else + expected_size = tctx->final_expected_size; + + /* + * Report missing chunks with language matching language used for reporting + * extraneous chunks. Mention the sizes expected for the missing chunks so + * the user can reconcile that against any extraneous chunk reports. + */ + if (last_missing > first_missing + 1 && + expected_size < TOAST_MAX_CHUNK_SIZE) + report_toast_corruption(hctx, ta, + psprintf("toast value %u missing chunks %d through %d with expected size %d and chunk %d with expected size %d", + ta->toast_pointer.va_valueid, + first_missing, last_missing - 1, + (int)TOAST_MAX_CHUNK_SIZE, + last_missing, expected_size)); + else if (last_missing == first_missing + 1 && + expected_size < TOAST_MAX_CHUNK_SIZE) + report_toast_corruption(hctx, ta, + psprintf("toast value %u missing chunk %d with expected size %d and chunk %d with expected size %d", + ta->toast_pointer.va_valueid, + first_missing, + (int)TOAST_MAX_CHUNK_SIZE, + last_missing, expected_size)); + else if (last_missing > first_missing) + report_toast_corruption(hctx, ta, + psprintf("toast value %u missing chunks %d through %d with expected size %d", + ta->toast_pointer.va_valueid, + first_missing, last_missing, + (int)TOAST_MAX_CHUNK_SIZE)); + else + report_toast_corruption(hctx, ta, + psprintf("toast value %u missing chunk %d with expected size %d", + ta->toast_pointer.va_valueid, + last_missing, expected_size)); +} + +/* + * Check the current toast tuple, recording any corruption found in + * ctx->tupstore. * * This is not equivalent to running verify_heapam on the toast table itself, * and is not hardened against corruption of the toast table. Rather, when @@ -1159,38 +1332,73 @@ check_tuple_visibility(HeapCheckContext *ctx) * each toast tuple being checked against where we are in the sequence, as well * as each toast tuple having its varlena structure sanity checked. * - * Returns whether the toast tuple passed the corruption checks. + * Returns the size of the current toast tuple chunk, or zero if the + * chunk is not sufficiently sensible for the chunk size to be determined. */ -static void -check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, - ToastedAttribute *ta, int32 chunkno, int32 endchunk) +static int32 +check_toast_tuple(HeapTuple toasttup, ToastedAttribute *ta, + ToastCheckContext *tctx, HeapCheckContext *hctx, + int32 chunkno) { int32 curchunk; Pointer chunk; bool isnull; int32 chunksize; - int32 expected_size; + int32 max_valid_prior_chunk; /* * Have a chunk, extract the sequence number and the data */ curchunk = DatumGetInt32(fastgetattr(toasttup, 2, - ctx->toast_rel->rd_att, &isnull)); + hctx->toast_rel->rd_att, &isnull)); if (isnull) { - report_toast_corruption(ctx, ta, + report_toast_corruption(hctx, ta, psprintf("toast value %u has toast chunk with null sequence number", ta->toast_pointer.va_valueid)); - return; + return 0; } + + /* + * Maximum chunk sequence number in the expected range which is less than + * curchunk. Note that curchunk itself may be outside the valid range. + */ + max_valid_prior_chunk = Min(curchunk-1, tctx->final_expected_chunk); + + /* Report any missing chunks at the beginning of the expected sequence */ + if (chunkno == 0 && max_valid_prior_chunk >= 0) + report_missing_chunks(hctx, ta, tctx, 0, max_valid_prior_chunk); + + /* Complain if the chunk sequence number retreats */ + if (chunkno > 0 && curchunk < tctx->last_chunk_seen) + report_toast_corruption(hctx, ta, + psprintf("toast value %u index scan returned chunk number %d after chunk number %d", + ta->toast_pointer.va_valueid, + curchunk, tctx->last_chunk_seen)); + + /* Complain if the same chunk sequence number is returned multiple times */ + else if (chunkno > 0 && curchunk == tctx->last_chunk_seen) + report_toast_corruption(hctx, ta, + psprintf("toast value %u index scan returned chunk number %d more than once", + ta->toast_pointer.va_valueid, + curchunk)); + + /* Report any missing chunks in the middle of the expected sequence */ + else if (chunkno > 0 && max_valid_prior_chunk > tctx->last_chunk_seen) + report_missing_chunks(hctx, ta, tctx, tctx->last_chunk_seen + 1, + max_valid_prior_chunk); + + /* Remember this chunk sequence number as the last one seen */ + tctx->last_chunk_seen = curchunk; + chunk = DatumGetPointer(fastgetattr(toasttup, 3, - ctx->toast_rel->rd_att, &isnull)); + hctx->toast_rel->rd_att, &isnull)); if (isnull) { - report_toast_corruption(ctx, ta, + report_toast_corruption(hctx, ta, psprintf("toast value %u chunk %d has null data", ta->toast_pointer.va_valueid, chunkno)); - return; + return 0; } if (!VARATT_IS_EXTENDED(chunk)) chunksize = VARSIZE(chunk) - VARHDRSZ; @@ -1206,41 +1414,34 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, /* should never happen */ uint32 header = ((varattrib_4b *) chunk)->va_4byte.va_header; - report_toast_corruption(ctx, ta, + report_toast_corruption(hctx, ta, psprintf("toast value %u chunk %d has invalid varlena header %0x", ta->toast_pointer.va_valueid, chunkno, header)); - return; + return 0; } - /* - * Some checks on the data we've found - */ - if (curchunk != chunkno) - { - report_toast_corruption(ctx, ta, - psprintf("toast value %u chunk %d has sequence number %d, but expected sequence number %d", - ta->toast_pointer.va_valueid, - chunkno, curchunk, chunkno)); - return; - } - if (chunkno > endchunk) - { - report_toast_corruption(ctx, ta, - psprintf("toast value %u chunk %d follows last expected chunk %d", - ta->toast_pointer.va_valueid, - chunkno, endchunk)); - return; - } + /* Report an extraneous chunk outside the expected sequence */ + if (curchunk < 0 || curchunk > tctx->final_expected_chunk) + handle_extraneous_chunk(hctx, ta, tctx, curchunk, chunksize); - expected_size = curchunk < endchunk ? TOAST_MAX_CHUNK_SIZE - : VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - (endchunk * TOAST_MAX_CHUNK_SIZE); + /* Report a partial chunk before the final expected chunk */ + else if (curchunk < tctx->final_expected_chunk && chunksize != TOAST_MAX_CHUNK_SIZE) + report_toast_corruption(hctx, ta, + psprintf("toast value %u chunk %d has size %d, but expected chunk with size %d", + ta->toast_pointer.va_valueid, + curchunk, chunksize, + (int)TOAST_MAX_CHUNK_SIZE)); - if (chunksize != expected_size) - report_toast_corruption(ctx, ta, - psprintf("toast value %u chunk %d has size %u, but expected size %u", + /* Report a final chunk of the wrong size */ + else if (curchunk == tctx->final_expected_chunk && chunksize != tctx->final_expected_size) + report_toast_corruption(hctx, ta, + psprintf("toast value %u chunk %d has size %d, but expected chunk with size %d", ta->toast_pointer.va_valueid, - chunkno, chunksize, expected_size)); + curchunk, chunksize, + tctx->final_expected_size)); + + return chunksize; } /* @@ -1379,14 +1580,55 @@ check_tuple_attribute(HeapCheckContext *ctx) */ VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); + /* Oversized toasted attributes should never be stored */ + if (toast_pointer.va_rawsize > VARLENA_SIZE_LIMIT) + report_corruption(ctx, + psprintf("toast value %u rawsize %u exceeds limit %u", + toast_pointer.va_valueid, + toast_pointer.va_rawsize, + VARLENA_SIZE_LIMIT)); + + /* 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)); + + /* Compressed attributes should have a valid compression method */ + if (VARATT_IS_COMPRESSED(&toast_pointer)) + { + ToastCompressionId cmid; + bool invalid = true; + + cmid = TOAST_COMPRESS_METHOD(&toast_pointer); + switch (cmid) + { + /* List of all valid compression method IDs */ + case TOAST_PGLZ_COMPRESSION_ID: + case TOAST_LZ4_COMPRESSION_ID: + invalid = false; + break; + + /* Recognized but invalid compression method ID */ + case TOAST_INVALID_COMPRESSION_ID: + break; + + /* Intentionally no default here */ + } + + if (invalid) + report_corruption(ctx, + psprintf("toast value %u has invalid compression method id %d", + toast_pointer.va_valueid, cmid)); + } + /* The tuple header better claim to contain toasted values */ if (!(infomask & HEAP_HASEXTERNAL)) - { report_corruption(ctx, psprintf("toast value %u is external but tuple header flag HEAP_HASEXTERNAL not set", toast_pointer.va_valueid)); - return true; - } /* The relation better have a toast table */ if (!ctx->rel->rd_rel->reltoastrelid) @@ -1397,6 +1639,14 @@ 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)); + /* If we were told to skip toast checking, then we're done. */ if (ctx->toast_rel == NULL) return true; @@ -1437,9 +1687,20 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) bool found_toasttup; HeapTuple toasttup; int32 chunkno; - int32 endchunk; + int64 totalsize; /* corrupt toast could overflow 32 bits */ + int32 extsize; + ToastCheckContext tctx; + + /* Calculate expected number of chunks and size of final chunk */ + extsize = VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer); + tctx.final_expected_chunk = (extsize - 1) / TOAST_MAX_CHUNK_SIZE; + tctx.final_expected_size = extsize - tctx.final_expected_chunk * TOAST_MAX_CHUNK_SIZE; - endchunk = (VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - 1) / TOAST_MAX_CHUNK_SIZE; + /* Have not yet seen any chunks for this toast tuple */ + tctx.have_extraneous_chunks = false; + tctx.first_extraneous = -1; + tctx.last_extraneous = -1; + tctx.last_chunk_seen = -1; /* * Setup a scan key to find chunks in toast table with matching va_valueid @@ -1459,13 +1720,14 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) &SnapshotToast, 1, &toastkey); chunkno = 0; + totalsize = 0; found_toasttup = false; while ((toasttup = systable_getnext_ordered(toastscan, ForwardScanDirection)) != NULL) { found_toasttup = true; - check_toast_tuple(toasttup, ctx, ta, chunkno, endchunk); + totalsize += check_toast_tuple(toasttup, ta, &tctx, ctx, chunkno); chunkno++; } systable_endscan_ordered(toastscan); @@ -1474,11 +1736,15 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) report_toast_corruption(ctx, ta, psprintf("toast value %u not found in toast table", ta->toast_pointer.va_valueid)); - else if (chunkno != (endchunk + 1)) + else if (chunkno != tctx.final_expected_chunk + 1 || extsize != totalsize) report_toast_corruption(ctx, ta, - psprintf("toast value %u was expected to end at chunk %d, but ended at chunk %d", + psprintf("toast value %u was expected to end at chunk %u with total size %d, but ended at chunk %u with total size " INT64_FORMAT, ta->toast_pointer.va_valueid, - (endchunk + 1), chunkno)); + (tctx.final_expected_chunk + 1), + extsize, chunkno, totalsize)); + + /* Report any remaining cached extraneous chunks */ + report_extraneous_chunks(ctx, ta, &tctx, NULL, NULL); } /* diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index c7aff677d4..996ef03180 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2558,6 +2558,7 @@ TimestampTz TmFromChar TmToChar ToastAttrInfo +ToastCheckContext ToastTupleContext ToastedAttribute TocEntry -- 2.21.1 (Apple Git-122.3)