Hi Postgres hackers,
I found a case where CRC of 1Gb block is calculated first and then immediately
discarded.

There is a limit on WAL record size - XLogRecordMaxSize. If the record
being inserted is larger than that, it is discarded and error is reported:

ERROR:  oversized WAL record
DETAIL:  WAL record would be 1069547521 bytes (of maximum 1069547520 bytes)

However, crc of record data is calculated before the record size is validated,
and in case of oversized record this crc is not used anywhere.

It is surely a minor issue, but might be worth fixing. I'm proposing a patch.
Since this situation is not covered by any tests I also included a test case
for failing on huge WAL records.
---
Sergey Fukanchik
From 1795b6eca263db9a8a941f27c7ebae911954dcff Mon Sep 17 00:00:00 2001
From: Sergey Fukanchik <s.fukanc...@postgrespro.ru>
Date: Sat, 6 Sep 2025 12:40:01 +0300
Subject: [PATCH] Perform check for oversized WAL record before calculating
 record CRC

---
 src/backend/access/transam/xloginsert.c       | 26 +++++++++----------
 .../regress/expected/oversized_wal_record.out |  5 ++++
 src/test/regress/parallel_schedule            |  2 +-
 src/test/regress/sql/oversized_wal_record.sql |  4 +++
 4 files changed, 23 insertions(+), 14 deletions(-)
 create mode 100644 src/test/regress/expected/oversized_wal_record.out
 create mode 100644 src/test/regress/sql/oversized_wal_record.sql

diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index c7571429e8e..e8c55924c12 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -904,19 +904,6 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
 	hdr_rdt.len = (scratch - hdr_scratch);
 	total_len += hdr_rdt.len;
 
-	/*
-	 * Calculate CRC of the data
-	 *
-	 * Note that the record header isn't added into the CRC initially since we
-	 * don't know the prev-link yet.  Thus, the CRC will represent the CRC of
-	 * the whole record in the order: rdata, then backup blocks, then record
-	 * header.
-	 */
-	INIT_CRC32C(rdata_crc);
-	COMP_CRC32C(rdata_crc, hdr_scratch + SizeOfXLogRecord, hdr_rdt.len - SizeOfXLogRecord);
-	for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
-		COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
-
 	/*
 	 * Ensure that the XLogRecord is not too large.
 	 *
@@ -930,6 +917,19 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
 				 errdetail_internal("WAL record would be %" PRIu64 " bytes (of maximum %u bytes); rmid %u flags %u.",
 									total_len, XLogRecordMaxSize, rmid, info)));
 
+	/*
+	 * Calculate CRC of the data
+	 *
+	 * Note that the record header isn't added into the CRC initially since we
+	 * don't know the prev-link yet.  Thus, the CRC will represent the CRC of
+	 * the whole record in the order: rdata, then backup blocks, then record
+	 * header.
+	 */
+	INIT_CRC32C(rdata_crc);
+	COMP_CRC32C(rdata_crc, hdr_scratch + SizeOfXLogRecord, hdr_rdt.len - SizeOfXLogRecord);
+	for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
+		COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
+
 	/*
 	 * Fill in the fields in the record header. Prev-link is filled in later,
 	 * once we know where in the WAL the record will be inserted. The CRC does
diff --git a/src/test/regress/expected/oversized_wal_record.out b/src/test/regress/expected/oversized_wal_record.out
new file mode 100644
index 00000000000..c9da04b59ec
--- /dev/null
+++ b/src/test/regress/expected/oversized_wal_record.out
@@ -0,0 +1,5 @@
+-- Try to create a WAL record which is larger than the limit.
+-- That should raise an error.
+select pg_logical_emit_message(false, 'a', repeat('00000000', (1020 * 1024 * 1024)/8+1), true);
+ERROR:  oversized WAL record
+DETAIL:  WAL record would be 1069547583 bytes (of maximum 1069547520 bytes); rmid 21 flags 0.
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index fbffc67ae60..913e2583129 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -36,7 +36,7 @@ test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comment
 # execute two copy tests in parallel, to check that copy itself
 # is concurrent safe.
 # ----------
-test: copy copyselect copydml copyencoding insert insert_conflict
+test: copy copyselect copydml copyencoding insert insert_conflict oversized_wal_record
 
 # ----------
 # More groups of parallel tests
diff --git a/src/test/regress/sql/oversized_wal_record.sql b/src/test/regress/sql/oversized_wal_record.sql
new file mode 100644
index 00000000000..555b6035c0f
--- /dev/null
+++ b/src/test/regress/sql/oversized_wal_record.sql
@@ -0,0 +1,4 @@
+-- Try to create a WAL record which is larger than the limit.
+-- That should raise an error.
+
+select pg_logical_emit_message(false, 'a', repeat('00000000', (1020 * 1024 * 1024)/8+1), true);
-- 
2.34.1

Reply via email to