Fix edge-case for xl_tot_len broken by bae868c.

macdice · macdice · commit bde2f1847f51 · 2023-09-26T10:59:49.000+13:00
bae868c removed a check that was still needed. If you had an xl_tot_len at the end of a page that was too small for a record header, but not big enough to span onto the next page, we'd immediately perform the CRC check using a bogus large length. Because of arbitrary coding differences between the CRC implementations on different platforms, nothing very bad happened on common modern systems. On systems using the _sb8.c fallback we could segfault. Restore that check, add a new assertion and supply a test for that case. Back-patch to 12, like bae868c. Tested-by: Tom Lane <tgl@sss.pgh.pa.us> Tested-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLCkTT7zYjzOxuLGahBdQ%3DMcF%3Dz5ZvrjSOnW4EDhVjT-g%40mail.gmail.com
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
@@ -330,6 +330,15 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
 	}
 	else
 	{
+		/* There may be no next page if it's too small. */
+		if (total_len < SizeOfXLogRecord)
+		{
+			report_invalid_record(state,
+								  "invalid record length at %X/%X: wanted %u, got %u",
+								  (uint32) (RecPtr >> 32), (uint32) RecPtr,
+								  (uint32) SizeOfXLogRecord, total_len);
+			goto err;
+		}
 		/* We'll validate the header once we have the next page. */
 		gotheader = false;
 	}
@@ -745,6 +754,8 @@ ValidXLogRecord(XLogReaderState *state, XLogRecord *record, XLogRecPtr recptr)
 {
 	pg_crc32c	crc;
 
+	Assert(record->xl_tot_len >= SizeOfXLogRecord);
+
 	/* Calculate the CRC */
 	INIT_CRC32C(crc);
 	COMP_CRC32C(crc, ((char *) record) + SizeOfXLogRecord, record->xl_tot_len - SizeOfXLogRecord);
diff --git a/src/test/recovery/t/039_end_of_wal.pl b/src/test/recovery/t/039_end_of_wal.pl
@@ -287,6 +287,19 @@ sub advance_to_record_splitting_zone
 		$log_size),
 	"xl_tot_len short");
 
+# xl_tot_len in final position, not big enough to span into a new page but
+# also not eligible for regular record header validation
+emit_message($node, 0);
+$end_lsn = advance_to_record_splitting_zone($node);
+$node->stop('immediate');
+write_wal($node, $TLI, $end_lsn, build_record_header(1));
+$log_size = -s $node->logfile;
+$node->start;
+ok( $node->log_contains(
+		"invalid record length at .*: wanted 24, got 1", $log_size
+	),
+	"xl_tot_len short at end-of-page");
+
 # Need more pages, but xl_prev check fails first.
 emit_message($node, 0);
 $end_lsn = advance_out_of_record_splitting_zone($node);

Original file line number	Diff line number	Diff line change
`@@ -330,6 +330,15 @@ XLogReadRecord(XLogReaderState state, XLogRecPtr RecPtr, char *errormsg)`
`330`	`330`	`}`
`331`	`331`	`else`
`332`	`332`	`{`
	`333`	`+ /* There may be no next page if it's too small. */`
	`334`	`+ if (total_len < SizeOfXLogRecord)`
	`335`	`+ {`
	`336`	`+ report_invalid_record(state,`
	`337`	`+ "invalid record length at %X/%X: wanted %u, got %u",`
	`338`	`+ (uint32) (RecPtr >> 32), (uint32) RecPtr,`
	`339`	`+ (uint32) SizeOfXLogRecord, total_len);`
	`340`	`+ goto err;`
	`341`	`+ }`
`333`	`342`	`/* We'll validate the header once we have the next page. */`
`334`	`343`	`gotheader = false;`
`335`	`344`	`}`
`@@ -745,6 +754,8 @@ ValidXLogRecord(XLogReaderState state, XLogRecord record, XLogRecPtr recptr)`
`745`	`754`	`{`
`746`	`755`	`pg_crc32c crc;`
`747`	`756`
	`757`	`+ Assert(record->xl_tot_len >= SizeOfXLogRecord);`
	`758`	`+`
`748`	`759`	`/* Calculate the CRC */`
`749`	`760`	`INIT_CRC32C(crc);`
`750`	`761`	`COMP_CRC32C(crc, ((char *) record) + SizeOfXLogRecord, record->xl_tot_len - SizeOfXLogRecord);`