postgres.git / summary / log / commit / refs
commit 46c93b705644ad0746dfbd6e0b737fd03e41817f
Author: Michael Paquier <michael@paquier.xyz>
Date: Mon Mar 09 04:46:37 2026 +0000
Fix size underestimation of DSA pagemap for odd-sized segments
When make_new_segment() creates an odd-sized segment, the pagemap was
only sized based on a number of usable_pages entries, forgetting that a
segment also contains metadata pages, and that the FreePageManager uses
absolute page indices that cover the entire segment. This
miscalculation could cause accesses to pagemap entries to be out of
bounds. During subsequent reuse of the allocated segment, allocations
landing on pages with indices higher than usable_pages could cause
out-of-bounds pagemap reads and/or writes. On write, 'span' pointers
are stored into the data area, corrupting the allocated objects. On
read (aka during a dsa_free), garbage is interpreted as a span pointer,
typically crashing the server in dsa_get_address().
The normal geometric path correctly sizes the pagemap for all pages in
the segment. The odd-sized path needs to do the same, but it works
forward from usable_pages rather than backward from total_size.
This commit fixes the sizing of the odd-sized case by adding pagemap
entries for the metadata pages after the initial metadata_bytes
calculation, using an integer ceiling division to compute the exact
number of additional entries needed in one go, avoiding any iteration in
the calculation.
An assertion is added in the code path for odd-sized segments, ensuring
that the pagemap includes the metadata area, and that the result is
appropriately sized.
This problem would show up depending on the size requested for the
allocation of a DSA segment. The reporter has noticed this issue when a
parallel hash join makes a DSA allocation large enough to trigger the
odd-sized segment path, but it could happen for anything that does a DSA
allocation.
A regression test is added to test_dsa, down to v17 where the test
module has been introduced. This adds a set of cheap tests to check the
problem, the new assertion being useful for this purpose. Sami has
proposed a test that took a longer time than what I have done here; the
test committed is faster and good enough to check the odd-sized
allocation path.
Author: Paul Bunn <paul.bunn@icloud.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/044401dcabac$fe432490$fac96db0$@icloud.com
Backpatch-through: 14
src/backend/utils/mmgr/dsa.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c
index c3c69bd3894..a1ca2c8b46c 100644
--- a/src/backend/utils/mmgr/dsa.c
+++ b/src/backend/utils/mmgr/dsa.c
@@ -2117,6 +2117,8 @@ make_new_segment(dsa_area *area, size_t requested_pages)
/* See if that is enough... */
if (requested_pages > usable_pages)
{
+ size_t total_requested_pages PG_USED_FOR_ASSERTS_ONLY;
+
/*
* We'll make an odd-sized segment, working forward from the requested
* number of pages.
@@ -2127,10 +2129,37 @@ make_new_segment(dsa_area *area, size_t requested_pages)
MAXALIGN(sizeof(FreePageManager)) +
usable_pages * sizeof(dsa_pointer);
+ /*
+ * We must also account for pagemap entries needed to cover the
+ * metadata pages themselves. The pagemap must track all pages in the
+ * segment, including the pages occupied by metadata.
+ *
+ * This formula uses integer ceiling division to compute the exact
+ * number of additional entries needed. The divisor (FPM_PAGE_SIZE -
+ * sizeof(dsa_pointer)) accounts for the fact that each metadata page
+ * consumes one pagemap entry of sizeof(dsa_pointer) bytes, leaving
+ * only (FPM_PAGE_SIZE - sizeof(dsa_pointer)) net bytes per metadata
+ * page.
+ */
+ metadata_bytes +=
+ ((metadata_bytes + (FPM_PAGE_SIZE - sizeof(dsa_pointer)) - 1) /
+ (FPM_PAGE_SIZE - sizeof(dsa_pointer))) *
+ sizeof(dsa_pointer);
+
/* Add padding up to next page boundary. */
if (metadata_bytes % FPM_PAGE_SIZE != 0)
metadata_bytes += FPM_PAGE_SIZE - (metadata_bytes % FPM_PAGE_SIZE);
total_size = metadata_bytes + usable_pages * FPM_PAGE_SIZE;
+ total_requested_pages = total_size / FPM_PAGE_SIZE;
+
+ /*
+ * Verify that we allocated enough pagemap entries for metadata and
+ * usable pages. This reverse-engineers the new calculation of
+ * "metadata_bytes" done based on the new "requested_pages" for an
+ * odd-sized segment.
+ */
+ Assert((metadata_bytes - MAXALIGN(sizeof(dsa_segment_header)) -
+ MAXALIGN(sizeof(FreePageManager))) / sizeof(dsa_pointer) >= total_requested_pages);
/* Is that too large for dsa_pointer's addressing scheme? */
if (total_size > DSA_MAX_SEGMENT_SIZE)
[parent: 42734f296615]