Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b9aa416

Browse files
committed
Avoid integer overflow while testing wal_skip_threshold condition.
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 c6b9204 which introduced wal_skip_threshold, so back-patch to v13. Discussion: https://postgr.es/m/1a01f0-66ec2d80-3b-68487680@27595217 Backpatch-through: 13
1 parent a5358c1 commit b9aa416

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

src/backend/catalog/storage.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
763763
{
764764
ForkNumber fork;
765765
BlockNumber nblocks[MAX_FORKNUM + 1];
766-
BlockNumber total_blocks = 0;
766+
uint64 total_blocks = 0;
767767
SMgrRelation srel;
768768

769769
srel = smgropen(pendingsync->rlocator, INVALID_PROC_NUMBER);
@@ -807,7 +807,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
807807
* main fork is longer than ever but FSM fork gets shorter.
808808
*/
809809
if (pendingsync->is_truncated ||
810-
total_blocks * BLCKSZ / 1024 >= wal_skip_threshold)
810+
total_blocks >= wal_skip_threshold * (uint64) 1024 / BLCKSZ)
811811
{
812812
/* allocate the initial array, or extend it, if needed */
813813
if (maxrels == 0)

0 commit comments

Comments
 (0)