Improve our ability to detect bogus pointers passed to pfree et al.

Commit c6e0fe1f2 was a shade too trusting that any pointer passed
to pfree, repalloc, etc will point at a valid chunk.  Notably,
passing a pointer that was actually obtained from malloc tended
to result in obscure assertion failures, if not worse.  (On FreeBSD
I've seen such mistakes take down the entire cluster, seemingly as
a result of clobbering shared memory.)

To improve matters, extend the mcxt_methods[] array so that it
has entries for every possible MemoryContextMethodID bit-pattern,
with the currently unassigned ID codes pointing to error-reporting
functions.  Then, fiddle with the ID assignments so that patterns
likely to be associated with bad pointers aren't valid ID codes.
In particular, we should avoid assigning bit patterns 000 (zeroed
memory) and 111 (wipe_mem'd memory).

It turns out that on glibc (Linux), malloc uses chunk headers that
have flag bits in the same place we keep MemoryContextMethodID,
and that the bit patterns 000, 001, 010 are the only ones we'll
see as long as the backend isn't threaded.  So we can have very
robust detection of pfree'ing a malloc-assigned block on that
platform, at least so long as we can refrain from using up those
ID codes.  On other platforms, we don't have such a good guarantee,
but keeping 000 reserved will be enough to catch many such cases.

While here, make GetMemoryChunkMethodID() local to mcxt.c, as there
seems no need for it to be exposed even in memutils_internal.h.

Patch by me, with suggestions from Andres Freund and David Rowley.

Discussion: https://postgr.es/m/2910981.1665080361@sss.pgh.pa.us

1 files changed, 14 insertions, 26 deletions

diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h
index 377cee7a84..bc2cbdd506 100644
--- a/src/include/utils/memutils_internal.h
+++ b/src/include/utils/memutils_internal.h
@@ -74,15 +74,26 @@ extern void SlabCheck(MemoryContext context);
  * MemoryContextMethodID
  *		A unique identifier for each MemoryContext implementation which
  *		indicates the index into the mcxt_methods[] array. See mcxt.c.
- *		The maximum value for this enum is constrained by
- *		MEMORY_CONTEXT_METHODID_MASK.  If an enum value higher than that is
- *		required then MEMORY_CONTEXT_METHODID_BITS will need to be increased.
+ *
+ * For robust error detection, ensure that MemoryContextMethodID has a value
+ * for each possible bit-pattern of MEMORY_CONTEXT_METHODID_MASK, and make
+ * dummy entries for unused IDs in the mcxt_methods[] array.  We also try
+ * to avoid using bit-patterns as valid IDs if they are likely to occur in
+ * garbage data, or if they could falsely match on chunks that are really from
+ * malloc not palloc.  (We can't tell that for most malloc implementations,
+ * but it happens that glibc stores flag bits in the same place where we put
+ * the MemoryContextMethodID, so the possible values are predictable for it.)
  */
 typedef enum MemoryContextMethodID
 {
+	MCTX_UNUSED1_ID,			/* 000 occurs in never-used memory */
+	MCTX_UNUSED2_ID,			/* glibc malloc'd chunks usually match 001 */
+	MCTX_UNUSED3_ID,			/* glibc malloc'd chunks > 128kB match 010 */
 	MCTX_ASET_ID,
 	MCTX_GENERATION_ID,
 	MCTX_SLAB_ID,
+	MCTX_UNUSED4_ID,			/* available */
+	MCTX_UNUSED5_ID				/* 111 occurs in wipe_mem'd memory */
 } MemoryContextMethodID;
 
 /*
@@ -104,27 +115,4 @@ extern void MemoryContextCreate(MemoryContext node,
 								MemoryContext parent,
 								const char *name);
 
-/*
- * GetMemoryChunkMethodID
- *		Return the MemoryContextMethodID from the uint64 chunk header which
- *		directly precedes 'pointer'.
- */
-static inline MemoryContextMethodID
-GetMemoryChunkMethodID(void *pointer)
-{
-	uint64		header;
-
-	/*
-	 * Try to detect bogus pointers handed to us, poorly though we can.
-	 * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-	 * allocated chunk.
-	 */
-	Assert(pointer != NULL);
-	Assert(pointer == (void *) MAXALIGN(pointer));
-
-	header = *((uint64 *) ((char *) pointer - sizeof(uint64)));
-
-	return (MemoryContextMethodID) (header & MEMORY_CONTEXT_METHODID_MASK);
-}
-
 #endif							/* MEMUTILS_INTERNAL_H */