| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: Avoid integer overflow while testing wal_skip_threshold conditio |
| Date: | 2025-01-30 20:36:54 |
| Message-ID: | E1tdbHS-004UFB-ME@gemulon.postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
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
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/b9aa4166fa3823d4f1f76286ca21fcfa991ce036
Modified Files
--------------
src/backend/catalog/storage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2025-01-30 21:44:53 | pgsql: Use "ssize_t" not "long" in max_stack_depth-related code. |
| Previous Message | Tom Lane | 2025-01-30 20:36:37 | pgsql: Avoid integer overflow while testing wal_skip_threshold conditio |