pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Avoid integer overflow while testing wal_skip_threshold conditio
Date: Thu, 30 Jan 2025 20:36:37 +0000
Message-ID: <E1tdbHA-004UEE-V5@gemulon.postgresql.org> (raw)

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 c6b92041d which introduced wal_skip_threshold,
so back-patch to v13.

Discussion: https://postgr.es/m/1a01f0-66ec2d80-3b-68487680@27595217
Backpatch-through: 13

Branch
------
REL_13_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/49a38238e2b63b24c0b183bdf35866de546f5fef

Modified Files
--------------
src/backend/catalog/storage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1tdbHA-004UEE-V5@gemulon.postgresql.org>
Permalink:  ../E1tdbHA-004UEE-V5@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tdbHA-004UEE-V5@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Avoid integer overflow while testing wal_skip_threshold conditio
  In-Reply-To: <E1tdbHA-004UEE-V5@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox