[PATCH] Perform check for oversized WAL record before calculating record CRC

Lists: pgsql-hackers
From: Sergey Fukanchik <s(dot)fukanchik(at)postgrespro(dot)ru>
To: pgsql-hackers(at)postgresql(dot)org
Subject: [PATCH] Perform check for oversized WAL record before calculating record CRC
Date: 2025-09-06 11:00:46
Message-ID: db2c6c76-3ff0-484f-9957-11b99732d943@postgrespro.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Perform-check-for-oversized-WAL-record-before-calcul.patch text/x-patch 4.0 KB

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Sergey Fukanchik <s(dot)fukanchik(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Perform check for oversized WAL record before calculating record CRC
Date: 2025-09-06 17:30:52
Message-ID: 509B5624-091E-45F9-B0D9-84DDE2C6CB94@yandex-team.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

> On 6 Sep 2025, at 16:00, Sergey Fukanchik <s(dot)fukanchik(at)postgrespro(dot)ru> wrote:
>
> <0001-Perform-check-for-oversized-WAL-record-before-calcul.patch>

Hi Sergey!

It seems to me reasonable to move size check above CRC computation. However, it seems suspicious to me to run a test that allocates 1Gb in `make check`. Maybe, there are places that are not exercised too often. Perhaps recovery tests or something like that.

Best regards, Andrey Borodin.


From: Sergey Fukanchik <s(dot)fukanchik(at)postgrespro(dot)ru>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Perform check for oversized WAL record before calculating record CRC
Date: 2025-09-07 14:00:56
Message-ID: 6ee042d1-7456-4732-b1e8-602b6beafb9a@postgrespro.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

> It seems to me reasonable to move size check above CRC computation. However, it seems suspicious to me to run a test that allocates 1Gb in `make check`. Maybe, there are places that are not exercised too often. Perhaps recovery tests or something like that.

Hi Andrey,

I share your concern about memory consumption and also agree recovery
tests look like the right place for this test.

So I split the patch into two - the change proper and the converted TAP
test, guarded by PG_TEST_EXTRA. Attaching both patches.

---

Sergey

Attachment Content-Type Size
v2-0001-Perform-check-for-oversized-WAL-record-before-cal.patch text/x-patch 2.2 KB
v2-0002-Add-a-TAP-test-for-oversized-WAL-records.patch text/x-patch 1.9 KB