From 6ec89610925448b1e7e9377afa73e158b291693c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 1 Sep 2022 19:22:35 +1200 Subject: [PATCH] Fix some possibly latent bugs in slab.c Primarily, this fixes an incorrect calculation in SlabCheck which was looking in the wrong byte for the sentinel check. The reason that we've never noticed this before in the form of a failing sentinel check is because the pre-check to this always fails because all current core users of slab contexts have a chunk size which is already MAXALIGNed, therefore there's never any space for the sentinel byte. It is possible that an extension needs to use a slab context and if they do with a chunk size that's not MAXALIGNed, then they'll likely get errors about overwritten sentinel bytes. Additionally, this patch changes various calculations which are being done based on the sizeof(SlabBlock). Currently, sizeof(SlabBlock) is a multiple of 8, therefore sizeof(SlabBlock) is the same as MAXALIGN(sizeof(SlabBlock)), however, if we were to ever have to add any fields to that struct as part of a bug fix, then SlabAlloc could end up returning a non-MAXALIGNed pointer. To be safe, let's ensure we always MAXALIGN sizeof(SlabBlock) before using it in any calculations. This patch has already been applied to master in d5ee4db0e. Diagnosed-by: Tomas Vondra, Tom Lane Author: Tomas Vondra, David Rowley Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com Backpatch-through: 10 --- src/backend/utils/mmgr/slab.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 553dd7f667..0f08ce3ce6 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -56,6 +56,8 @@ #include "utils/memdebug.h" #include "utils/memutils.h" +#define Slab_BLOCKHDRSZ MAXALIGN(sizeof(SlabBlock)) + /* * SlabContext is a specialized implementation of MemoryContext. */ @@ -116,10 +118,10 @@ typedef struct SlabChunk #define SlabChunkGetPointer(chk) \ ((void *)(((char *)(chk)) + sizeof(SlabChunk))) #define SlabBlockGetChunk(slab, block, idx) \ - ((SlabChunk *) ((char *) (block) + sizeof(SlabBlock) \ + ((SlabChunk *) ((char *) (block) + Slab_BLOCKHDRSZ \ + (idx * slab->fullChunkSize))) #define SlabBlockStart(block) \ - ((char *) block + sizeof(SlabBlock)) + ((char *) block + Slab_BLOCKHDRSZ) #define SlabChunkIndex(slab, block, chunk) \ (((char *) chunk - SlabBlockStart(block)) / slab->fullChunkSize) @@ -169,7 +171,7 @@ static const MemoryContextMethods SlabMethods = { * chunkSize: allocation chunk size * * The chunkSize may not exceed: - * MAXALIGN_DOWN(SIZE_MAX) - MAXALIGN(sizeof(SlabBlock)) - sizeof(SlabChunk) + * MAXALIGN_DOWN(SIZE_MAX) - MAXALIGN(Slab_BLOCKHDRSZ) - sizeof(SlabChunk) */ MemoryContext SlabContextCreate(MemoryContext parent, @@ -199,12 +201,12 @@ SlabContextCreate(MemoryContext parent, fullChunkSize = sizeof(SlabChunk) + MAXALIGN(chunkSize); /* Make sure the block can store at least one chunk. */ - if (blockSize < fullChunkSize + sizeof(SlabBlock)) + if (blockSize < fullChunkSize + Slab_BLOCKHDRSZ) elog(ERROR, "block size %zu for slab is too small for %zu chunks", blockSize, chunkSize); /* Compute maximum number of chunks per block */ - chunksPerBlock = (blockSize - sizeof(SlabBlock)) / fullChunkSize; + chunksPerBlock = (blockSize - Slab_BLOCKHDRSZ) / fullChunkSize; /* The freelist starts with 0, ends with chunksPerBlock. */ freelistSize = sizeof(dlist_head) * (chunksPerBlock + 1); @@ -772,7 +774,7 @@ SlabCheck(MemoryContext context) /* there might be sentinel (thanks to alignment) */ if (slab->chunkSize < (slab->fullChunkSize - sizeof(SlabChunk))) - if (!sentinel_ok(chunk, slab->chunkSize)) + if (!sentinel_ok(chunk, sizeof(SlabChunk) + slab->chunkSize)) elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p", name, block, chunk); }