From 71e0624cec05b8e8835b28f3dbd58a8aa2896f41 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sat, 2 May 2026 23:09:00 +0200 Subject: [PATCH v3 1/3] valgrind: Add NOACCESS sentinels for shm_toc entries When built with USE_VALGRIND add a couple NOACCESS bytes after each entry allocated in a shm_toc from the segment. Since markings are not shared between processes they are either set when a process allocates or when a process looks up an allocation in the toc. But as we have not saved the size of an allocation in the toc we need to, on lookup, base where we set the sentinel on where the start of the following allocation is so any padding will cause a gap before the sentinel. This is far from ideal but better than nothing. We also need to make sure to clear all NOACCESS bytes when detaching from a DSM segment since a shm_toc does not know when it was destroyed or deatched. Author: Tomas Vondra Author: Andreas Karlsson --- src/backend/storage/ipc/dsm.c | 7 ++++++ src/backend/storage/ipc/shm_toc.c | 39 +++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c index 8b69df4ff26..f447d1543e3 100644 --- a/src/backend/storage/ipc/dsm.c +++ b/src/backend/storage/ipc/dsm.c @@ -45,6 +45,7 @@ #include "storage/shmem.h" #include "storage/subsystems.h" #include "utils/freepage.h" +#include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/resowner.h" @@ -846,6 +847,12 @@ dsm_detach(dsm_segment *seg) */ if (seg->mapped_address != NULL) { + /* + * We need to clear up NOACCESS regions set by shm_toc as shm_tocs + * have no function for deatching or destroying. + */ + VALGRIND_MAKE_MEM_DEFINED(seg->mapped_address, seg->mapped_size); + if (!is_main_region_dsm_handle(seg->handle)) dsm_impl_op(DSM_OP_DETACH, seg->handle, 0, &seg->impl_private, &seg->mapped_address, &seg->mapped_size, WARNING); diff --git a/src/backend/storage/ipc/shm_toc.c b/src/backend/storage/ipc/shm_toc.c index 2217d48c3d9..0157f5c586c 100644 --- a/src/backend/storage/ipc/shm_toc.c +++ b/src/backend/storage/ipc/shm_toc.c @@ -16,6 +16,7 @@ #include "port/atomics.h" #include "storage/shm_toc.h" #include "storage/spin.h" +#include "utils/memdebug.h" typedef struct shm_toc_entry { @@ -74,6 +75,13 @@ shm_toc_attach(uint64 magic, void *address) return toc; } +/* With valgrind, we want to add a couple NOACCESS bytes */ +#ifdef USE_VALGRIND +#define NUM_NOACCESS_BYTES 32 +#else +#define NUM_NOACCESS_BYTES 0 +#endif + /* * Allocate shared memory from a segment managed by a table of contents. * @@ -87,6 +95,7 @@ shm_toc_attach(uint64 magic, void *address) void * shm_toc_allocate(shm_toc *toc, Size nbytes) { + Size reqbytes; Size total_bytes; Size allocated_bytes; Size nentry; @@ -98,7 +107,7 @@ shm_toc_allocate(shm_toc *toc, Size nbytes) * proper definition for the minimum to make atomic ops safe, but * BUFFERALIGN ought to be enough. */ - nbytes = BUFFERALIGN(nbytes); + reqbytes = BUFFERALIGN(nbytes + NUM_NOACCESS_BYTES); SpinLockAcquire(&toc->toc_mutex); @@ -109,18 +118,24 @@ shm_toc_allocate(shm_toc *toc, Size nbytes) + allocated_bytes; /* Check for memory exhaustion and overflow. */ - if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes) + if (toc_bytes + reqbytes > total_bytes || toc_bytes + reqbytes < toc_bytes) { SpinLockRelease(&toc->toc_mutex); ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of shared memory"))); } - toc->toc_allocated_bytes += nbytes; + toc->toc_allocated_bytes += reqbytes; SpinLockRelease(&toc->toc_mutex); - return ((char *) toc) + (total_bytes - allocated_bytes - nbytes); +#ifdef USE_VALGRIND + /* Make the bytes at the end no-access */ + VALGRIND_MAKE_MEM_NOACCESS(((char *) toc) + (total_bytes - allocated_bytes - reqbytes + nbytes), + reqbytes - nbytes); +#endif + + return ((char *) toc) + (total_bytes - allocated_bytes - reqbytes); } /* @@ -249,7 +264,20 @@ shm_toc_lookup(shm_toc *toc, uint64 key, bool noError) for (i = 0; i < nentry; ++i) { if (toc->toc_entry[i].key == key) + { +#ifdef USE_VALGRIND + /* + * Since we do not know the size of entries we can only use the + * start of the next entry for setting the no-access sentinel. + */ + Size nextoffset = i == 0 ? toc->toc_total_bytes : toc->toc_entry[i - 1].offset; + + VALGRIND_MAKE_MEM_NOACCESS(((char *) toc) + nextoffset - NUM_NOACCESS_BYTES, + NUM_NOACCESS_BYTES); +#endif + return ((char *) toc) + toc->toc_entry[i].offset; + } } /* No matching entry was found. */ @@ -272,5 +300,8 @@ shm_toc_estimate(shm_toc_estimator *e) sz = add_size(sz, mul_size(e->number_of_keys, sizeof(shm_toc_entry))); sz = add_size(sz, e->space_for_chunks); + /* add space for ENOACCESS bytes, NUM_NOACCESS_BYTES per key */ + sz = add_size(sz, mul_size(e->number_of_keys, NUM_NOACCESS_BYTES)); + return BUFFERALIGN(sz); } -- 2.54.0