v1-0001-Make-more-effort-to-have-a-sentinel-byte-in-memor.patch

From a61c1646987996192ef9e917ca0fabbef51e34c9 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Fri, 2 Sep 2022 13:30:06 +1200
Subject: [PATCH v1] Make more effort to have a sentinel byte in memory
 contexts

---
 src/backend/utils/mmgr/aset.c       | 41 +++++++++++++++++++--------
 src/backend/utils/mmgr/generation.c | 43 +++++++++++++++++------------
 src/backend/utils/mmgr/slab.c       | 37 +++++++++++++------------
 3 files changed, 74 insertions(+), 47 deletions(-)

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index b6eeb8abab..ec423375ae 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -705,7 +705,13 @@ AllocSetAlloc(MemoryContext context, Size size)
 	 */
 	if (size > set->allocChunkLimit)
 	{
+#ifdef MEMORY_CONTEXT_CHECKING
+		/* ensure there's always space for the sentinel byte */
+		chunk_size = MAXALIGN(size + 1);
+#else
 		chunk_size = MAXALIGN(size);
+#endif
+
 		blksize = chunk_size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
 		block = (AllocBlock) malloc(blksize);
 		if (block == NULL)
@@ -724,8 +730,8 @@ AllocSetAlloc(MemoryContext context, Size size)
 #ifdef MEMORY_CONTEXT_CHECKING
 		chunk->requested_size = size;
 		/* set mark to catch clobber of "unused" space */
-		if (size < chunk_size)
-			set_sentinel(MemoryChunkGetPointer(chunk), size);
+		Assert(size < chunk_size);
+		set_sentinel(MemoryChunkGetPointer(chunk), size);
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 		/* fill the allocated space with junk */
@@ -766,6 +772,12 @@ AllocSetAlloc(MemoryContext context, Size size)
 	 * corresponding free list to see if there is a free chunk we could reuse.
 	 * If one is found, remove it from the free list, make it again a member
 	 * of the alloc set and return its data address.
+	 *
+	 * Note that we don't attempt to ensure there's space for the sentinel
+	 * byte here.  We expect a large proportion of allocations to be for sizes
+	 * which are already a power of 2.  If we were to always make space for a
+	 * sentinel byte in MEMORY_CONTEXT_CHECKING builds, then we'd end up
+	 * doubling the memory requirements for such allocations.
 	 */
 	fidx = AllocSetFreeIndex(size);
 	chunk = set->freelist[fidx];
@@ -992,10 +1004,10 @@ AllocSetFree(void *pointer)
 			Size		chunk_size = block->endptr - (char *) pointer;
 
 			/* Test for someone scribbling on unused space in chunk */
-			if (chunk->requested_size < chunk_size)
-				if (!sentinel_ok(pointer, chunk->requested_size))
-					elog(WARNING, "detected write past chunk end in %s %p",
-						 set->header.name, chunk);
+			Assert(chunk->requested_size < chunk_size);
+			if (!sentinel_ok(pointer, chunk->requested_size))
+				elog(WARNING, "detected write past chunk end in %s %p",
+					 set->header.name, chunk);
 		}
 #endif
 
@@ -1098,10 +1110,10 @@ AllocSetRealloc(void *pointer, Size size)
 
 #ifdef MEMORY_CONTEXT_CHECKING
 		/* Test for someone scribbling on unused space in chunk */
-		if (chunk->requested_size < oldsize)
-			if (!sentinel_ok(pointer, chunk->requested_size))
-				elog(WARNING, "detected write past chunk end in %s %p",
-					 set->header.name, chunk);
+		Assert(chunk->requested_size < oldsize);
+		if (!sentinel_ok(pointer, chunk->requested_size))
+			elog(WARNING, "detected write past chunk end in %s %p",
+				 set->header.name, chunk);
 #endif
 
 		/*
@@ -1111,7 +1123,12 @@ AllocSetRealloc(void *pointer, Size size)
 		if (block->freeptr != block->endptr)
 			elog(ERROR, "could not find block containing chunk %p", chunk);
 
+#ifdef MEMORY_CONTEXT_CHECKING
+		/* ensure there's always space for the sentinel byte */
+		chksize = MAXALIGN(size + 1);
+#else
 		chksize = MAXALIGN(size);
+#endif
 
 		/* Do the realloc */
 		blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
@@ -1162,8 +1179,8 @@ AllocSetRealloc(void *pointer, Size size)
 
 		chunk->requested_size = size;
 		/* set mark to catch clobber of "unused" space */
-		if (size < chksize)
-			set_sentinel(pointer, size);
+		Assert(size < chksize);
+		set_sentinel(pointer, size);
 #else							/* !MEMORY_CONTEXT_CHECKING */
 
 		/*
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index b39894ec94..c743b24fa7 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -342,8 +342,16 @@ GenerationAlloc(MemoryContext context, Size size)
 	GenerationContext *set = (GenerationContext *) context;
 	GenerationBlock *block;
 	MemoryChunk *chunk;
-	Size		chunk_size = MAXALIGN(size);
-	Size		required_size = chunk_size + Generation_CHUNKHDRSZ;
+	Size		chunk_size;
+	Size		required_size;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+	/* ensure there's always space for the sentinel byte */
+	chunk_size = MAXALIGN(size + 1);
+#else
+	chunk_size = MAXALIGN(size);
+#endif
+	required_size = chunk_size + Generation_CHUNKHDRSZ;
 
 	/* is it an over-sized chunk? if yes, allocate special block */
 	if (chunk_size > set->allocChunkLimit)
@@ -373,8 +381,8 @@ GenerationAlloc(MemoryContext context, Size size)
 #ifdef MEMORY_CONTEXT_CHECKING
 		chunk->requested_size = size;
 		/* set mark to catch clobber of "unused" space */
-		if (size < chunk_size)
-			set_sentinel(MemoryChunkGetPointer(chunk), size);
+		Assert(size < chunk_size);
+		set_sentinel(MemoryChunkGetPointer(chunk), size);
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 		/* fill the allocated space with junk */
@@ -491,8 +499,8 @@ GenerationAlloc(MemoryContext context, Size size)
 #ifdef MEMORY_CONTEXT_CHECKING
 	chunk->requested_size = size;
 	/* set mark to catch clobber of "unused" space */
-	if (size < chunk_size)
-		set_sentinel(MemoryChunkGetPointer(chunk), size);
+	Assert(size < chunk_size);
+	set_sentinel(MemoryChunkGetPointer(chunk), size);
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
 	/* fill the allocated space with junk */
@@ -634,10 +642,10 @@ GenerationFree(void *pointer)
 
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* Test for someone scribbling on unused space in chunk */
-	if (chunk->requested_size < chunksize)
-		if (!sentinel_ok(pointer, chunk->requested_size))
-			elog(WARNING, "detected write past chunk end in %s %p",
-				 ((MemoryContext) block->context)->name, chunk);
+	Assert(chunk->requested_size < chunksize);
+	if (!sentinel_ok(pointer, chunk->requested_size))
+		elog(WARNING, "detected write past chunk end in %s %p",
+			 ((MemoryContext) block->context)->name, chunk);
 #endif
 
 #ifdef CLOBBER_FREED_MEMORY
@@ -727,10 +735,10 @@ GenerationRealloc(void *pointer, Size size)
 
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* Test for someone scribbling on unused space in chunk */
-	if (chunk->requested_size < oldsize)
-		if (!sentinel_ok(pointer, chunk->requested_size))
-			elog(WARNING, "detected write past chunk end in %s %p",
-				 ((MemoryContext) set)->name, chunk);
+	Assert(chunk->requested_size < oldsize);
+	if (!sentinel_ok(pointer, chunk->requested_size))
+		elog(WARNING, "detected write past chunk end in %s %p",
+			 ((MemoryContext) set)->name, chunk);
 #endif
 
 	/*
@@ -769,8 +777,7 @@ GenerationRealloc(void *pointer, Size size)
 									   oldsize - size);
 
 		/* set mark to catch clobber of "unused" space */
-		if (size < oldsize)
-			set_sentinel(pointer, size);
+		set_sentinel(pointer, size);
 #else							/* !MEMORY_CONTEXT_CHECKING */
 
 		/*
@@ -1034,8 +1041,8 @@ GenerationCheck(MemoryContext context)
 						 name, block, chunk);
 
 				/* check sentinel */
-				if (chunk->requested_size < chunksize &&
-					!sentinel_ok(chunk, Generation_CHUNKHDRSZ + chunk->requested_size))
+				Assert(chunk->requested_size < chunksize);
+				if (!sentinel_ok(chunk, Generation_CHUNKHDRSZ + chunk->requested_size))
 					elog(WARNING, "problem in Generation %s: detected write past chunk end in block %p, chunk %p",
 						 name, block, chunk);
 			}
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 2d70adef09..9149aaafcb 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -145,7 +145,12 @@ SlabContextCreate(MemoryContext parent,
 		chunkSize = sizeof(int);
 
 	/* chunk, including SLAB header (both addresses nicely aligned) */
+#ifdef MEMORY_CONTEXT_CHECKING
+	/* ensure there's always space for the sentinel byte */
+	fullChunkSize = Slab_CHUNKHDRSZ + MAXALIGN(chunkSize + 1);
+#else
 	fullChunkSize = Slab_CHUNKHDRSZ + MAXALIGN(chunkSize);
+#endif
 
 	/* Make sure the block can store at least one chunk. */
 	if (blockSize < fullChunkSize + Slab_BLOCKHDRSZ)
@@ -420,14 +425,12 @@ SlabAlloc(MemoryContext context, Size size)
 						  MCTX_SLAB_ID);
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* slab mark to catch clobber of "unused" space */
-	if (slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ))
-	{
-		set_sentinel(MemoryChunkGetPointer(chunk), size);
-		VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) +
-								   Slab_CHUNKHDRSZ + slab->chunkSize,
-								   slab->fullChunkSize -
-								   (slab->chunkSize + Slab_CHUNKHDRSZ));
-	}
+	Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
+	set_sentinel(MemoryChunkGetPointer(chunk), size);
+	VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) +
+							   Slab_CHUNKHDRSZ + slab->chunkSize,
+							   slab->fullChunkSize -
+							   (slab->chunkSize + Slab_CHUNKHDRSZ));
 #endif
 
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
@@ -454,10 +457,10 @@ SlabFree(void *pointer)
 
 #ifdef MEMORY_CONTEXT_CHECKING
 	/* Test for someone scribbling on unused space in chunk */
-	if (slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ))
-		if (!sentinel_ok(pointer, slab->chunkSize))
-			elog(WARNING, "detected write past chunk end in %s %p",
-				 slab->header.name, chunk);
+	Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
+	if (!sentinel_ok(pointer, slab->chunkSize))
+		elog(WARNING, "detected write past chunk end in %s %p",
+			 slab->header.name, chunk);
 #endif
 
 	/* compute index of the chunk with respect to block start */
@@ -744,11 +747,11 @@ SlabCheck(MemoryContext context)
 						elog(WARNING, "problem in slab %s: bogus block link in block %p, chunk %p",
 							 name, block, chunk);
 
-					/* there might be sentinel (thanks to alignment) */
-					if (slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ))
-						if (!sentinel_ok(chunk, Slab_CHUNKHDRSZ + slab->chunkSize))
-							elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p",
-								 name, block, chunk);
+					/* check the sentinel byte is intact */
+					Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
+					if (!sentinel_ok(chunk, Slab_CHUNKHDRSZ + slab->chunkSize))
+						elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p",
+							 name, block, chunk);
 				}
 			}
 
-- 
2.34.1