From 9982b1277976415216c90f80155c35aa5021576a Mon Sep 17 00:00:00 2001 From: Chris Jefferson Date: Mon, 5 Feb 2018 15:44:04 +0000 Subject: [PATCH 1/3] Add sanity checking asserts to gasman --- src/gasman.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/gasman.c b/src/gasman.c index cee5766110..5787c7e225 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -300,6 +300,7 @@ void CHANGED_BAG(Bag bag) { static inline UInt SpaceBetweenPointers(Bag * a, Bag * b) { + GAP_ASSERT(b <= a); UInt res = (((UInt)((UInt)(a) - (UInt)(b))) / sizeof(Bag)); return res; } @@ -312,6 +313,16 @@ static inline UInt SpaceBetweenPointers(Bag * a, Bag * b) #define SizeAllBagsArea SpaceBetweenPointers(AllocBags, OldBags) #define SizeWorkspace SpaceBetweenPointers(EndBags, MptrBags) +#if defined(GAP_KERNEL_DEBUG) +static int SanityCheckGasmanPointers(void) +{ + return MptrBags <= OldBags && + OldBags <= YoungBags && + YoungBags <= AllocBags && + AllocBags <= EndBags; +} +#endif + /**************************************************************************** ** *V FreeMptrBags . . . . . . . . . . . . . . . list of free bag identifiers @@ -1032,6 +1043,7 @@ void InitBags ( /* Set ChangedBags to a proper initial value */ ChangedBags = 0; + GAP_ASSERT(SanityCheckGasmanPointers()); } @@ -1117,6 +1129,8 @@ Bag NewBag ( /* set the masterpointer */ SET_PTR_BAG(bag, DATA(header)); + GAP_ASSERT(SanityCheckGasmanPointers()); + /* return the identifier of the new bag */ return bag; } @@ -1372,6 +1386,7 @@ UInt ResizeBag ( sizeof(Obj) * WORDS_BAG(old_size)); } + GAP_ASSERT(SanityCheckGasmanPointers()); /* return success */ return 1; } @@ -1650,6 +1665,7 @@ UInt CollectBags ( UInt done; /* do we have to make a full gc */ UInt i; /* loop variable */ + GAP_ASSERT(SanityCheckGasmanPointers()); CANARY_DISABLE_VALGRIND(); CLEAR_CANARY(); #ifdef DEBUG_MASTERPOINTERS @@ -2092,6 +2108,8 @@ UInt CollectBags ( CANARY_ENABLE_VALGRIND(); + GAP_ASSERT(SanityCheckGasmanPointers()); + /* return success */ return 1; } From b5da0cebc8510f4624bee37bae7ad7d070a15132 Mon Sep 17 00:00:00 2001 From: Chris Jefferson Date: Mon, 5 Feb 2018 15:44:44 +0000 Subject: [PATCH 2/3] Catch overflows and stop functions returning in gasman Catch a couple of cases where we can cause pointer overflow. Also no callers of NewBag or ResizeBag check the return value, so instead of returning 0 on failure exit GAP. --- src/gasman.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/gasman.c b/src/gasman.c index 5787c7e225..0e8be7b0a4 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -116,6 +116,7 @@ #include #include +#include /**************************************************************************** ** @@ -298,7 +299,7 @@ void CHANGED_BAG(Bag bag) { answer in units of a word (ie sizeof(UInt) bytes), which should therefore be small enough not to cause problems. */ -static inline UInt SpaceBetweenPointers(Bag * a, Bag * b) +static inline UInt SpaceBetweenPointers(const Bag * a, const Bag * b) { GAP_ASSERT(b <= a); UInt res = (((UInt)((UInt)(a) - (UInt)(b))) / sizeof(Bag)); @@ -1096,7 +1097,7 @@ Bag NewBag ( if ( (FreeMptrBags == 0 || SizeAllocationArea < WORDS_BAG(sizeof(BagHeader)+size)) && CollectBags( size, 0 ) == 0 ) { - return 0; + SyAbortBags("cannot extend the workspace any more!!!!"); } #ifdef COUNT_BAGS @@ -1302,9 +1303,9 @@ UInt ResizeBag ( else if (CONST_PTR_BAG(bag) + WORDS_BAG(old_size) == AllocBags) { CLEAR_CANARY(); // check that enough storage for the new bag is available - if (EndBags < CONST_PTR_BAG(bag) + WORDS_BAG(new_size) + if (SpaceBetweenPointers(EndBags, CONST_PTR_BAG(bag)) < WORDS_BAG(new_size) && CollectBags( new_size-old_size, 0 ) == 0 ) { - return 0; + SyAbortBags("cannot extend the workspace any more!!!!!"); } // update header pointer in case bag moved @@ -1332,7 +1333,7 @@ UInt ResizeBag ( /* check that enough storage for the new bag is available */ if ( SizeAllocationArea < WORDS_BAG(sizeof(BagHeader)+new_size) && CollectBags( new_size, 0 ) == 0 ) { - return 0; + SyAbortBags("Cannot extend the workspace any more!!!!!!"); } CLEAR_CANARY(); @@ -1952,6 +1953,11 @@ UInt CollectBags ( /* * * * * * * * * * * * * * * check phase * * * * * * * * * * * * * * */ + // Check if this allocation would even fit into memory + if (SIZE_MAX - (size_t)(sizeof(BagHeader) + size) < (size_t)AllocBags) { + return 0; + } + // store in 'stopBags' where this allocation takes us Bag * stopBags = AllocBags + WORDS_BAG(sizeof(BagHeader)+size); From c0bb87f23f67778dbed16ae766cbaaabd71659c0 Mon Sep 17 00:00:00 2001 From: Chris Jefferson Date: Mon, 5 Feb 2018 22:05:06 +0000 Subject: [PATCH 3/3] Fix incorrect use of SpaceBetweenPointers StopBags may be smaller or larger than EndBags, so we cannot use SpaceBetweenPointers. We switch to just using '-'. --- src/gasman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gasman.c b/src/gasman.c index 0e8be7b0a4..5fcfad8262 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -2089,7 +2089,7 @@ UInt CollectBags ( /* information after the check phase */ if ( MsgsFuncBags ) (*MsgsFuncBags)( FullBags, 5, - SpaceBetweenPointers(EndBags, stopBags)/(1024/sizeof(Bag))); + (EndBags - stopBags)/(1024/sizeof(Bag))); if ( MsgsFuncBags ) (*MsgsFuncBags)( FullBags, 6, SizeWorkspace/(1024/sizeof(Bag)));