Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0e87dfe

Browse files
committed
Harden memory context allocators against bogus chunk pointers.
Before commitc6e0fe1, functions such as AllocSetFree could prettysafely presume that they were given a valid chunk pointer for theirown type of context, because the indirect call through a memorycontext object and method struct would be very unlikely to workotherwise. But now, if pfree() is mistakenly invoked on a pointerto garbage, we have three chances in eight of ending up at one ofthese functions. That means we need to take extra measures toverify that we are looking at what we're supposed to be looking at,especially in debug builds.Hence, add code to verify that the chunk's back-link to a block headerleads to a memory context object that satisfies the right sort ofIsA() check. This is still a bit weaker than what we did before,but for the moment assume that an IsA() check is sufficient.As a compromise between speed and safety, implement these checksas Asserts when dealing with small chunks but plain test-and-elogswhen dealing with large (external) chunks. The latter case shouldnot be too performance-critical, but the former case probably is.In slab.c, all chunks are small; but nonetheless use a plain testin SlabRealloc, because that is certainly not performance-critical,indeed we should be suspicious that it's being called in error.In aset.c, additionally add some assertions that the "value" fieldof the chunk header is within the small range allowed for freelistindexes. Without that, we might find ourselves trying to wipemost of memory when CLOBBER_FREED_MEMORY is enabled, or scribblingon a "freelist header" that's far away from the context object.Eventually, field experience might show us that it's smarter forthese tests to be active always, but for now we'll try to getaway with just having them as assertions.While at it, also be more uniform about asserting that contextobjects passed as parameters are of the type we expect. Someplaces missed that altogether, and slab.c was for no very goodreason doing it differently from the other allocators.Discussion:https://postgr.es/m/3578387.1665244345@sss.pgh.pa.us
1 parent235eb4d commit0e87dfe

File tree

3 files changed

+187
-51
lines changed

3 files changed

+187
-51
lines changed

‎src/backend/utils/mmgr/aset.c

Lines changed: 84 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ typedef struct AllocFreeListLink
132132
#defineGetFreeListLink(chkptr) \
133133
(AllocFreeListLink *) ((char *) (chkptr) + ALLOC_CHUNKHDRSZ)
134134

135+
/* Validate a freelist index retrieved from a chunk header */
136+
#defineFreeListIdxIsValid(fidx) \
137+
((fidx) >= 0 && (fidx) < ALLOCSET_NUM_FREELISTS)
138+
135139
/* Determine the size of the chunk based on the freelist index */
136140
#defineGetChunkSizeFromFreeListIdx(fidx) \
137141
((((Size) 1) << ALLOC_MINBITS) << (fidx))
@@ -202,7 +206,15 @@ typedef struct AllocBlockData
202206
* AllocSetIsValid
203207
*True iff set is valid allocation set.
204208
*/
205-
#defineAllocSetIsValid(set) PointerIsValid(set)
209+
#defineAllocSetIsValid(set) \
210+
(PointerIsValid(set) && IsA(set, AllocSetContext))
211+
212+
/*
213+
* AllocBlockIsValid
214+
*True iff block is valid block of allocation set.
215+
*/
216+
#defineAllocBlockIsValid(block) \
217+
(PointerIsValid(block) && AllocSetIsValid((block)->aset))
206218

207219
/*
208220
* We always store external chunks on a dedicated block. This makes fetching
@@ -530,8 +542,7 @@ AllocSetReset(MemoryContext context)
530542
{
531543
AllocSetset= (AllocSet)context;
532544
AllocBlockblock;
533-
SizekeepersizePG_USED_FOR_ASSERTS_ONLY
534-
=set->keeper->endptr- ((char*)set);
545+
SizekeepersizePG_USED_FOR_ASSERTS_ONLY;
535546

536547
AssertArg(AllocSetIsValid(set));
537548

@@ -540,6 +551,9 @@ AllocSetReset(MemoryContext context)
540551
AllocSetCheck(context);
541552
#endif
542553

554+
/* Remember keeper block size for Assert below */
555+
keepersize=set->keeper->endptr- ((char*)set);
556+
543557
/* Clear chunk freelists */
544558
MemSetAligned(set->freelist,0,sizeof(set->freelist));
545559

@@ -598,8 +612,7 @@ AllocSetDelete(MemoryContext context)
598612
{
599613
AllocSetset= (AllocSet)context;
600614
AllocBlockblock=set->blocks;
601-
SizekeepersizePG_USED_FOR_ASSERTS_ONLY
602-
=set->keeper->endptr- ((char*)set);
615+
SizekeepersizePG_USED_FOR_ASSERTS_ONLY;
603616

604617
AssertArg(AllocSetIsValid(set));
605618

@@ -608,6 +621,9 @@ AllocSetDelete(MemoryContext context)
608621
AllocSetCheck(context);
609622
#endif
610623

624+
/* Remember keeper block size for Assert below */
625+
keepersize=set->keeper->endptr- ((char*)set);
626+
611627
/*
612628
* If the context is a candidate for a freelist, put it into that freelist
613629
* instead of destroying it.
@@ -994,9 +1010,16 @@ AllocSetFree(void *pointer)
9941010

9951011
if (MemoryChunkIsExternal(chunk))
9961012
{
997-
1013+
/* Release single-chunk block. */
9981014
AllocBlockblock=ExternalChunkGetBlock(chunk);
9991015

1016+
/*
1017+
* Try to verify that we have a sane block pointer: the block header
1018+
* should reference an aset and the freeptr should match the endptr.
1019+
*/
1020+
if (!AllocBlockIsValid(block)||block->freeptr!=block->endptr)
1021+
elog(ERROR,"could not find block containing chunk %p",chunk);
1022+
10001023
set=block->aset;
10011024

10021025
#ifdefMEMORY_CONTEXT_CHECKING
@@ -1011,14 +1034,6 @@ AllocSetFree(void *pointer)
10111034
}
10121035
#endif
10131036

1014-
1015-
/*
1016-
* Try to verify that we have a sane block pointer, the freeptr should
1017-
* match the endptr.
1018-
*/
1019-
if (block->freeptr!=block->endptr)
1020-
elog(ERROR,"could not find block containing chunk %p",chunk);
1021-
10221037
/* OK, remove block from aset's list and free it */
10231038
if (block->prev)
10241039
block->prev->next=block->next;
@@ -1036,12 +1051,23 @@ AllocSetFree(void *pointer)
10361051
}
10371052
else
10381053
{
1039-
intfidx=MemoryChunkGetValue(chunk);
10401054
AllocBlockblock=MemoryChunkGetBlock(chunk);
1041-
AllocFreeListLink*link=GetFreeListLink(chunk);
1055+
intfidx;
1056+
AllocFreeListLink*link;
10421057

1058+
/*
1059+
* In this path, for speed reasons we just Assert that the referenced
1060+
* block is good. We can also Assert that the value field is sane.
1061+
* Future field experience may show that these Asserts had better
1062+
* become regular runtime test-and-elog checks.
1063+
*/
1064+
AssertArg(AllocBlockIsValid(block));
10431065
set=block->aset;
10441066

1067+
fidx=MemoryChunkGetValue(chunk);
1068+
Assert(FreeListIdxIsValid(fidx));
1069+
link=GetFreeListLink(chunk);
1070+
10451071
#ifdefMEMORY_CONTEXT_CHECKING
10461072
/* Test for someone scribbling on unused space in chunk */
10471073
if (chunk->requested_size<GetChunkSizeFromFreeListIdx(fidx))
@@ -1089,6 +1115,7 @@ AllocSetRealloc(void *pointer, Size size)
10891115
AllocSetset;
10901116
MemoryChunk*chunk=PointerGetMemoryChunk(pointer);
10911117
Sizeoldsize;
1118+
intfidx;
10921119

10931120
/* Allow access to private part of chunk header. */
10941121
VALGRIND_MAKE_MEM_DEFINED(chunk,ALLOCCHUNK_PRIVATE_LEN);
@@ -1105,9 +1132,18 @@ AllocSetRealloc(void *pointer, Size size)
11051132
Sizeoldblksize;
11061133

11071134
block=ExternalChunkGetBlock(chunk);
1108-
oldsize=block->endptr- (char*)pointer;
1135+
1136+
/*
1137+
* Try to verify that we have a sane block pointer: the block header
1138+
* should reference an aset and the freeptr should match the endptr.
1139+
*/
1140+
if (!AllocBlockIsValid(block)||block->freeptr!=block->endptr)
1141+
elog(ERROR,"could not find block containing chunk %p",chunk);
1142+
11091143
set=block->aset;
11101144

1145+
oldsize=block->endptr- (char*)pointer;
1146+
11111147
#ifdefMEMORY_CONTEXT_CHECKING
11121148
/* Test for someone scribbling on unused space in chunk */
11131149
Assert(chunk->requested_size<oldsize);
@@ -1116,13 +1152,6 @@ AllocSetRealloc(void *pointer, Size size)
11161152
set->header.name,chunk);
11171153
#endif
11181154

1119-
/*
1120-
* Try to verify that we have a sane block pointer, the freeptr should
1121-
* match the endptr.
1122-
*/
1123-
if (block->freeptr!=block->endptr)
1124-
elog(ERROR,"could not find block containing chunk %p",chunk);
1125-
11261155
#ifdefMEMORY_CONTEXT_CHECKING
11271156
/* ensure there's always space for the sentinel byte */
11281157
chksize=MAXALIGN(size+1);
@@ -1201,9 +1230,20 @@ AllocSetRealloc(void *pointer, Size size)
12011230
}
12021231

12031232
block=MemoryChunkGetBlock(chunk);
1204-
oldsize=GetChunkSizeFromFreeListIdx(MemoryChunkGetValue(chunk));
1233+
1234+
/*
1235+
* In this path, for speed reasons we just Assert that the referenced
1236+
* block is good. We can also Assert that the value field is sane. Future
1237+
* field experience may show that these Asserts had better become regular
1238+
* runtime test-and-elog checks.
1239+
*/
1240+
AssertArg(AllocBlockIsValid(block));
12051241
set=block->aset;
12061242

1243+
fidx=MemoryChunkGetValue(chunk);
1244+
Assert(FreeListIdxIsValid(fidx));
1245+
oldsize=GetChunkSizeFromFreeListIdx(fidx);
1246+
12071247
#ifdefMEMORY_CONTEXT_CHECKING
12081248
/* Test for someone scribbling on unused space in chunk */
12091249
if (chunk->requested_size<oldsize)
@@ -1328,6 +1368,7 @@ AllocSetGetChunkContext(void *pointer)
13281368
else
13291369
block= (AllocBlock)MemoryChunkGetBlock(chunk);
13301370

1371+
AssertArg(AllocBlockIsValid(block));
13311372
set=block->aset;
13321373

13331374
return&set->header;
@@ -1342,16 +1383,19 @@ Size
13421383
AllocSetGetChunkSpace(void*pointer)
13431384
{
13441385
MemoryChunk*chunk=PointerGetMemoryChunk(pointer);
1386+
intfidx;
13451387

13461388
if (MemoryChunkIsExternal(chunk))
13471389
{
13481390
AllocBlockblock=ExternalChunkGetBlock(chunk);
13491391

1392+
AssertArg(AllocBlockIsValid(block));
13501393
returnblock->endptr- (char*)chunk;
13511394
}
13521395

1353-
returnGetChunkSizeFromFreeListIdx(MemoryChunkGetValue(chunk))+
1354-
ALLOC_CHUNKHDRSZ;
1396+
fidx=MemoryChunkGetValue(chunk);
1397+
Assert(FreeListIdxIsValid(fidx));
1398+
returnGetChunkSizeFromFreeListIdx(fidx)+ALLOC_CHUNKHDRSZ;
13551399
}
13561400

13571401
/*
@@ -1361,6 +1405,8 @@ AllocSetGetChunkSpace(void *pointer)
13611405
bool
13621406
AllocSetIsEmpty(MemoryContextcontext)
13631407
{
1408+
AssertArg(AllocSetIsValid(context));
1409+
13641410
/*
13651411
* For now, we say "empty" only if the context is new or just reset. We
13661412
* could examine the freelists to determine if all space has been freed,
@@ -1394,6 +1440,8 @@ AllocSetStats(MemoryContext context,
13941440
AllocBlockblock;
13951441
intfidx;
13961442

1443+
AssertArg(AllocSetIsValid(set));
1444+
13971445
/* Include context header in totalspace */
13981446
totalspace=MAXALIGN(sizeof(AllocSetContext));
13991447

@@ -1405,14 +1453,14 @@ AllocSetStats(MemoryContext context,
14051453
}
14061454
for (fidx=0;fidx<ALLOCSET_NUM_FREELISTS;fidx++)
14071455
{
1456+
Sizechksz=GetChunkSizeFromFreeListIdx(fidx);
14081457
MemoryChunk*chunk=set->freelist[fidx];
14091458

14101459
while (chunk!=NULL)
14111460
{
1412-
Sizechksz=GetChunkSizeFromFreeListIdx(MemoryChunkGetValue(chunk));
14131461
AllocFreeListLink*link=GetFreeListLink(chunk);
14141462

1415-
Assert(GetChunkSizeFromFreeListIdx(fidx)==chksz);
1463+
Assert(MemoryChunkGetValue(chunk)==fidx);
14161464

14171465
freechunks++;
14181466
freespace+=chksz+ALLOC_CHUNKHDRSZ;
@@ -1522,7 +1570,13 @@ AllocSetCheck(MemoryContext context)
15221570
}
15231571
else
15241572
{
1525-
chsize=GetChunkSizeFromFreeListIdx(MemoryChunkGetValue(chunk));/* aligned chunk size */
1573+
intfidx=MemoryChunkGetValue(chunk);
1574+
1575+
if (!FreeListIdxIsValid(fidx))
1576+
elog(WARNING,"problem in alloc set %s: bad chunk size for chunk %p in block %p",
1577+
name,chunk,block);
1578+
1579+
chsize=GetChunkSizeFromFreeListIdx(fidx);/* aligned chunk size */
15261580

15271581
/*
15281582
* Check the stored block offset correctly references this

‎src/backend/utils/mmgr/generation.c

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,20 @@ struct GenerationBlock
105105
* simplicity.
106106
*/
107107
#defineGENERATIONCHUNK_PRIVATE_LENoffsetof(MemoryChunk, hdrmask)
108+
108109
/*
109110
* GenerationIsValid
110-
*True iff set is valid allocation set.
111+
*True iff set is valid generation set.
112+
*/
113+
#defineGenerationIsValid(set) \
114+
(PointerIsValid(set) && IsA(set, GenerationContext))
115+
116+
/*
117+
* GenerationBlockIsValid
118+
*True iff block is valid block of generation set.
111119
*/
112-
#defineGenerationIsValid(set) PointerIsValid(set)
120+
#defineGenerationBlockIsValid(block) \
121+
(PointerIsValid(block) && GenerationIsValid((block)->context))
113122

114123
/*
115124
* We always store external chunks on a dedicated block. This makes fetching
@@ -345,6 +354,8 @@ GenerationAlloc(MemoryContext context, Size size)
345354
Sizechunk_size;
346355
Sizerequired_size;
347356

357+
AssertArg(GenerationIsValid(set));
358+
348359
#ifdefMEMORY_CONTEXT_CHECKING
349360
/* ensure there's always space for the sentinel byte */
350361
chunk_size=MAXALIGN(size+1);
@@ -625,13 +636,29 @@ GenerationFree(void *pointer)
625636
if (MemoryChunkIsExternal(chunk))
626637
{
627638
block=ExternalChunkGetBlock(chunk);
639+
640+
/*
641+
* Try to verify that we have a sane block pointer: the block header
642+
* should reference a generation context.
643+
*/
644+
if (!GenerationBlockIsValid(block))
645+
elog(ERROR,"could not find block containing chunk %p",chunk);
646+
628647
#if defined(MEMORY_CONTEXT_CHECKING)|| defined(CLOBBER_FREED_MEMORY)
629648
chunksize=block->endptr- (char*)pointer;
630649
#endif
631650
}
632651
else
633652
{
634653
block=MemoryChunkGetBlock(chunk);
654+
655+
/*
656+
* In this path, for speed reasons we just Assert that the referenced
657+
* block is good. Future field experience may show that this Assert
658+
* had better become a regular runtime test-and-elog check.
659+
*/
660+
AssertArg(GenerationBlockIsValid(block));
661+
635662
#if defined(MEMORY_CONTEXT_CHECKING)|| defined(CLOBBER_FREED_MEMORY)
636663
chunksize=MemoryChunkGetValue(chunk);
637664
#endif
@@ -723,11 +750,27 @@ GenerationRealloc(void *pointer, Size size)
723750
if (MemoryChunkIsExternal(chunk))
724751
{
725752
block=ExternalChunkGetBlock(chunk);
753+
754+
/*
755+
* Try to verify that we have a sane block pointer: the block header
756+
* should reference a generation context.
757+
*/
758+
if (!GenerationBlockIsValid(block))
759+
elog(ERROR,"could not find block containing chunk %p",chunk);
760+
726761
oldsize=block->endptr- (char*)pointer;
727762
}
728763
else
729764
{
730765
block=MemoryChunkGetBlock(chunk);
766+
767+
/*
768+
* In this path, for speed reasons we just Assert that the referenced
769+
* block is good. Future field experience may show that this Assert
770+
* had better become a regular runtime test-and-elog check.
771+
*/
772+
AssertArg(GenerationBlockIsValid(block));
773+
731774
oldsize=MemoryChunkGetValue(chunk);
732775
}
733776

@@ -845,6 +888,7 @@ GenerationGetChunkContext(void *pointer)
845888
else
846889
block= (GenerationBlock*)MemoryChunkGetBlock(chunk);
847890

891+
AssertArg(GenerationBlockIsValid(block));
848892
return&block->context->header;
849893
}
850894

@@ -863,6 +907,7 @@ GenerationGetChunkSpace(void *pointer)
863907
{
864908
GenerationBlock*block=ExternalChunkGetBlock(chunk);
865909

910+
AssertArg(GenerationBlockIsValid(block));
866911
chunksize=block->endptr- (char*)pointer;
867912
}
868913
else
@@ -881,6 +926,8 @@ GenerationIsEmpty(MemoryContext context)
881926
GenerationContext*set= (GenerationContext*)context;
882927
dlist_iteriter;
883928

929+
AssertArg(GenerationIsValid(set));
930+
884931
dlist_foreach(iter,&set->blocks)
885932
{
886933
GenerationBlock*block=dlist_container(GenerationBlock,node,iter.cur);
@@ -917,6 +964,8 @@ GenerationStats(MemoryContext context,
917964
Sizefreespace=0;
918965
dlist_iteriter;
919966

967+
AssertArg(GenerationIsValid(set));
968+
920969
/* Include context header in totalspace */
921970
totalspace=MAXALIGN(sizeof(GenerationContext));
922971

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp