Hi All, Heikki, SHMEM_ATTACH_UNKNOWN_SIZE can be passed as argument to ShmemRequestStruct() when the caller wants to attach to an existing shared memory structure, whose size it does not know, after the startup. If the shared memory structure it wants to attach to does not exist, the request should fail. But instead ProcessShmemRequestsAfterStartup() ended up creating the structure with size = -1. InitShmemIndexEntry() did not catch it and created a ShmemIndexEntry with size = SIZE_MAX since size is an unsigned integer. ShmemAllocRaw() did not catch the overflow in address arithmetic and ended up allocating the new structure overlapping the earlier structure which can potentially cause memory corruption.
Attached patch fixes ProcessShmemRequestsAfterStartup() throw an error in this case, adds an Assert() in InitShmemIndexEntry() to make sure that a request with unknown size never reaches it, makes ShmemAllocRaw() check for overflow, and documents use of SHMEM_ATTACH_UNKNOWN_SIZE. I found this problem when working on resizable shared structures where the size of the structure may have changed from its initial size and hence may not be known. It's good to defend our shared memory structures from a bug in extension code. -- Best Wishes, Ashutosh Bapat
From 1a3426774ca9a4f12f4102ec03225adde9976237 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Fri, 18 Sep 2026 11:32:08 +0530 Subject: [PATCH v20260918] Attaching to a non-existing shared memory area after startup SHMEM_ATTACH_UNKNOWN_SIZE can be passed as argument to ShmemRequestStruct() when the caller wants to attach to an existing shared memory structure, whose size it does not know, after the startup. If the shared memory structure it wants to attach to does not exist, the request should fail. But instead ProcessShmemRequestsAfterStartup() ended up creating the structure with size = -1. InitShmemIndexEntry() did not catch it and created a ShmemIndexEntry with size = SIZE_MAX since size is an unsigned integer. ShmemAllocRaw() did not catch the overflow in size arithmetic and ended up allocating the new structure overlapping the earlier structure which can potentially cause memory corruption. Fix this by making ProcessShmemRequestsAfterStartup() reject the requests with size = SHMEM_ATTACH_UNKNOWN_SIZE when the structure with the requested name does not exist. Add an Assert() in InitShmemIndexEntry() to make sure that a request with unknown size never reaches it. Also make ShmemAllocRaw() check for overflow in its size arithmetic, as defense-in-depth against a similar mistake silently corrupting memory in the future. Also document use of SHMEM_ATTACH_UNKNOWN_SIZE. Author: Ashutosh Bapat <[email protected]> Discussion: TBD --- doc/src/sgml/xfunc.sgml | 10 ++++++++ src/backend/storage/ipc/shmem.c | 23 +++++++++++++++++-- .../test_shmem/t/001_late_shmem_alloc.pl | 17 ++++++++++++++ src/test/modules/test_shmem/test_shmem.c | 11 ++++++--- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index a90dba0662f..02eb676bf1a 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3745,6 +3745,16 @@ my_shmem_init(void *arg) areas are left in an abandoned state and any attempt to attach or re-initialize them will fail until the server is restarted. </para> + <para> + A backend attaching to an already allocated shared memory area must + normally pass the same <literal>size</literal> that was used to allocate + it to <function>ShmemRequestStruct()</function>; a mismatch results in an + error. If the attaching backend does not know the size in advance, it can + pass <literal>SHMEM_ATTACH_UNKNOWN_SIZE</literal> instead, to skip this + cross-check. <literal>SHMEM_ATTACH_UNKNOWN_SIZE</literal> can only be + used to attach to an area that already exists; using it before allocating + the area results in an error. + </para> </sect3> <sect3 id="xfunc-shared-addin-dynamic"> diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index f971ee24192..93de3649045 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -131,6 +131,7 @@ #include <unistd.h> #include "access/slru.h" +#include "common/int.h" #include "fmgr.h" #include "funcapi.h" #include "miscadmin.h" @@ -535,6 +536,9 @@ InitShmemIndexEntry(ShmemRequest *request) size_t allocated_size; void *structPtr; + /* Size must be known at this point. */ + Assert(request->options->size != SHMEM_ATTACH_UNKNOWN_SIZE); + /* look it up in the shmem index */ index_entry = (ShmemIndexEnt *) hash_search(ShmemIndex, name, HASH_ENTER_NULL, &found); @@ -836,6 +840,8 @@ ShmemAllocNoError(Size size) * * Also sets *allocated_size to the number of bytes allocated, which will * be equal to the number requested plus any padding we choose to add. + * + * Returns NULL in case space can not be allocated. */ static void * ShmemAllocRaw(Size size, Size alignment, Size *allocated_size) @@ -866,8 +872,14 @@ ShmemAllocRaw(Size size, Size alignment, Size *allocated_size) rawStart = ShmemAllocator->free_offset; newStart = TYPEALIGN(alignment, rawStart); - newFree = newStart + size; - if (newFree <= ShmemSegHdr->totalsize) + /* + * Check for overflow in case SHMEM_ATTACH_UNKNOWN_SIZE made all the way + * here in an unlikely event. Avoid throwing an error while we hold a + * spinlock. Let the caller handle it instead. + */ + if (unlikely(pg_add_size_overflow(newStart, size, &newFree))) + newSpace = NULL; + else if (newFree <= ShmemSegHdr->totalsize) { newSpace = (char *) ShmemBase + newStart; ShmemAllocator->free_offset = newFree; @@ -1024,7 +1036,14 @@ ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks) found_any = true; } else + { + if (request->options->size == SHMEM_ATTACH_UNKNOWN_SIZE) + ereport(ERROR, + (errmsg("cannot attach to shared memory struct \"%s\" because it does not exist", + request->options->name), + errdetail("SHMEM_ATTACH_UNKNOWN_SIZE can only be used to attach to an existing shared memory structure."))); notfound_any = true; + } } if (found_any && notfound_any) elog(ERROR, "some of the requested shmem areas have already been initialized"); diff --git a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl index 9432b42181b..739e24bf299 100644 --- a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl +++ b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl @@ -31,6 +31,23 @@ cmp_ok($attach_count2, '>', $attach_count1, $node->stop; +### +# Test that requesting a shmem area with size = SHMEM_ATTACH_UNKNOWN_SIZE after +# startup fails. +### +$node->append_conf('postgresql.conf', "test_shmem.area_size = -1"); +$node->start; + +my (undef, undef, $stderr) = + $node->psql("postgres", "SELECT get_test_shmem_attach_count();"); +like( + $stderr, + qr/cannot attach to shared memory struct "test_shmem area" because it does not exist/, + "unknown size request for a nonexistent area fails"); + +$node->stop; +$node->adjust_conf('postgresql.conf', 'test_shmem.area_size', undef); + ### # Test allocating memory after startup in single-user mode ### diff --git a/src/test/modules/test_shmem/test_shmem.c b/src/test/modules/test_shmem/test_shmem.c index 6cf47dc8968..31dd585023e 100644 --- a/src/test/modules/test_shmem/test_shmem.c +++ b/src/test/modules/test_shmem/test_shmem.c @@ -35,12 +35,17 @@ typedef struct TestShmemData static TestShmemData *TestShmem; -#define MIN_TEST_AREA_BYTES sizeof(TestShmemData) -#define DEFAULT_TEST_AREA_BYTES MIN_TEST_AREA_BYTES +/* + * The minimum is set to SHMEM_ATTACH_UNKNOWN_SIZE (instead of + * sizeof(TestShmemData)) to test behaviour when ShmemRequestStruct() is + * passed SHMEM_ATTACH_UNKNOWN_SIZE for an area that does not exist yet. + */ +#define MIN_TEST_AREA_BYTES SHMEM_ATTACH_UNKNOWN_SIZE +#define DEFAULT_TEST_AREA_BYTES sizeof(TestShmemData) #define MAX_TEST_AREA_BYTES 1000000 static bool attached_or_initialized = false; -static int test_shmem_area_size = MIN_TEST_AREA_BYTES; +static int test_shmem_area_size = DEFAULT_TEST_AREA_BYTES; static bool test_shmem_guc_defined = false; static void test_shmem_request(void *arg); base-commit: 26a3c0a45cc6017ee7f334c0ef63292d5e101b58 -- 2.34.1
