From 86addc39596cf980a5850d9b1dba23fdaa927485 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Thu, 8 Apr 2021 10:09:59 -0700 Subject: [PATCH v19 2/3] 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 if a toast pointer indicates the data is compressed, that the toast pointer records a valid compression method. 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. 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 | 56 +++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 13f420d9ad..7b7b40f415 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -30,6 +30,8 @@ PG_FUNCTION_INFO_V1(verify_heapam); /* The number of columns in tuples returned by verify_heapam */ #define HEAPCHECK_RELATION_COLS 4 +#define VARLENA_SIZE_LIMIT 0x3FFFFFFF + /* * Despite the name, we use this for reporting problems with both XIDs and * MXIDs. @@ -1379,14 +1381,54 @@ check_tuple_attribute(HeapCheckContext *ctx) */ VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); + 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 +1439,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; -- 2.21.1 (Apple Git-122.3)