v20-0002-amcheck-adding-toast-pointer-corruption-checks.patch

From 7a78901dbebfc099208f041faf30e4707d4a0647 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Thu, 8 Apr 2021 10:09:59 -0700
Subject: [PATCH v20 2/2] 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.

Checking for missing or extraneous toast chunks for a toasted
attribute.  While at it, checking that the toast index scan returns
chunks in order.
---
 contrib/amcheck/verify_heapam.c  | 402 +++++++++++++++++++++++++++----
 src/tools/pgindent/typedefs.list |   1 +
 2 files changed, 351 insertions(+), 52 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 13f420d9ad..93315c1f24 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,184 @@ 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 less than
+ * 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 && partialchunk != NULL &&
+		tctx->last_extraneous == *partialchunk - 1)
+	{
+		/*
+		 * For brevity, combine or more contiguous full chunks ending with a
+		 * partial chunk into just one corruption report.
+		 */
+		if (tctx->first_extraneous < tctx->last_extraneous)
+			report_toast_corruption(hctx, ta,
+									psprintf("toast value %u unexpected chunks %d through %d each with size %d followed by chunk %d with size %d",
+										 ta->toast_pointer.va_valueid,
+										 tctx->first_extraneous,
+										 tctx->last_extraneous,
+										 (int)TOAST_MAX_CHUNK_SIZE,
+										 *partialchunk, *partialsize));
+		else
+			report_toast_corruption(hctx, ta,
+									psprintf("toast value %u unexpected chunk %d with size %d followed by chunk %d with size %d",
+										 ta->toast_pointer.va_valueid,
+										 tctx->first_extraneous,
+										 (int)TOAST_MAX_CHUNK_SIZE,
+										 *partialchunk, *partialsize));
+		tctx->have_extraneous_chunks = false;
+		return;
+	}
+
+	if (tctx->have_extraneous_chunks)
+	{
+		/*
+		 * Either the previously seen extraneous chunks are not contiguous with
+		 * the partial chunk being reported, or there is no partial chunk being
+		 * reported.
+		 */
+		if (tctx->first_extraneous < tctx->last_extraneous)
+			report_toast_corruption(hctx, ta,
+									psprintf("toast value %u 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 unexpected chunk %d with size %d",
+										 ta->toast_pointer.va_valueid,
+										 tctx->first_extraneous,
+										 (int)TOAST_MAX_CHUNK_SIZE));
+	}
+
+	if (partialchunk != NULL)
+		/* Report the partial chunk */
+		report_toast_corruption(hctx, ta,
+								psprintf("toast value %u unexpected chunk %d with size %d",
+										 ta->toast_pointer.va_valueid,
+										 *partialchunk, *partialsize));
+	tctx->have_extraneous_chunks = false;
+}
+
+/*
+ * 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 against the state tracked in ctx, recording
- * any corruption found in ctx->tupstore.
+ * 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 +1364,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 +1446,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 +1612,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 +1671,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 +1719,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;
 
-	endchunk = (VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - 1) / TOAST_MAX_CHUNK_SIZE;
+	/* 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;
+
+	/* 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 +1752,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 +1768,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 %u, but ended at chunk %u",
+								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)