database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-08 07:21:33 +03:00
Code

Avoid integer overflow while testing wal_skip_threshold condition.

Browse Source 
smgrDoPendingSyncs had two distinct risks of integer overflow while
deciding which way to ensure durability of a newly-created relation.
First, it accumulated the total size of all forks in a variable of
type BlockNumber (uint32).  While we restrict an individual fork's
size to fit in that, I don't believe there's such a restriction on
all of them added together.  Second, it proceeded to multiply the
sum by BLCKSZ, which most certainly could overflow a uint32.

(The exact expression is total_blocks * BLCKSZ / 1024.  The
compiler might choose to optimize that to total_blocks * 8,
which is not at quite as much risk of overflow as a literal
reading would be, but it's still wrong.)

If an overflow did occur it could lead to a poor choice to
shove a very large relation into WAL instead of fsync'ing it.
This wouldn't be fatal, but it could be inefficient.

Change total_blocks to uint64 which should be plenty, and
rearrange the comparison calculation to be overflow-safe.

I noticed this while looking for ramifications of the proposed
change in MAX_KILOBYTES.  It's not entirely clear to me why
wal_skip_threshold is limited to MAX_KILOBYTES in the
first place, but in any case this code is unsafe regardless
of the range of wal_skip_threshold.

Oversight in c6b92041d which introduced wal_skip_threshold,
so back-patch to v13.

Discussion: https://postgr.es/m/1a01f0-66ec2d80-3b-68487680@27595217
Backpatch-through: 13
This commit is contained in:
Tom Lane 2025-01-30 15:36:07 -05:00
parent 6e41e9e5e0
commit 1e25cdb214
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/backend/catalog/storage.c
View File

@ -763,7 +763,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
{ {
ForkNumber fork; ForkNumber fork;
BlockNumber nblocks[MAX_FORKNUM + 1]; BlockNumber nblocks[MAX_FORKNUM + 1];
BlockNumber total_blocks = 0; uint64 total_blocks = 0;
SMgrRelation srel; SMgrRelation srel;
srel = smgropen(pendingsync->rlocator, INVALID_PROC_NUMBER); srel = smgropen(pendingsync->rlocator, INVALID_PROC_NUMBER);
@ -807,7 +807,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
* main fork is longer than ever but FSM fork gets shorter. * main fork is longer than ever but FSM fork gets shorter.
*/ */
if (pendingsync->is_truncated || if (pendingsync->is_truncated ||
total_blocks * BLCKSZ / 1024 >= wal_skip_threshold) total_blocks >= wal_skip_threshold * (uint64) 1024 / BLCKSZ)
{ {
/* allocate the initial array, or extend it, if needed */ /* allocate the initial array, or extend it, if needed */
if (maxrels == 0) if (maxrels == 0)