From 1bd07482531e9fae514212e729ce94eb7b534f42 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Tue, 11 Sep 2012 22:31:19 +0200 Subject: [PATCH] Fix crash in MemoryManager::PerformMaintenance() sFreeAreaCount wasn't decremented after removing an area from sFreeAreas, thus causing the loop to continue until enountering and crashing on a NULL pointer after removing the last area. Introduce helper methods _PushFreeArea() and _PopFreeArea() to ensure this cannot easily happen again. Fixes ticket #8972. --- src/system/kernel/slab/MemoryManager.cpp | 20 +++++++------------- src/system/kernel/slab/MemoryManager.h | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/system/kernel/slab/MemoryManager.cpp b/src/system/kernel/slab/MemoryManager.cpp index 3a6ac26f55..1f1fa8e0ee 100644 --- a/src/system/kernel/slab/MemoryManager.cpp +++ b/src/system/kernel/slab/MemoryManager.cpp @@ -22,7 +22,6 @@ #include "kernel_debug_config.h" #include "ObjectCache.h" -#include "slab_private.h" //#define TRACE_MEMORY_MANAGER @@ -849,15 +848,13 @@ MemoryManager::PerformMaintenance() if (_AllocateArea(0, area) != B_OK) return; - _push(sFreeAreas, area); - if (++sFreeAreaCount > 2) + _PushFreeArea(area); + if (sFreeAreaCount > 2) sMaintenanceNeeded = true; } else { // free until we only have two free ones - while (sFreeAreaCount > 2) { - Area* area = _pop(sFreeAreas); - _FreeArea(area, true, 0); - } + while (sFreeAreaCount > 2) + _FreeArea(_PopFreeArea(), true, 0); if (sFreeAreaCount == 0) sMaintenanceNeeded = true; @@ -956,8 +953,7 @@ MemoryManager::_AllocateChunks(size_t chunkSize, uint32 chunkCount, return B_OK; if (sFreeAreas != NULL) { - _AddArea(_pop(sFreeAreas)); - sFreeAreaCount--; + _AddArea(_PopFreeArea()); _RequestMaintenance(); _GetChunks(metaChunkList, chunkSize, chunkCount, _metaChunk, _chunk); @@ -1409,16 +1405,14 @@ MemoryManager::_FreeArea(Area* area, bool areaRemoved, uint32 flags) // We want to keep one or two free areas as a reserve. if (sFreeAreaCount <= 1) { - _push(sFreeAreas, area); - sFreeAreaCount++; + _PushFreeArea(area); return; } if (area->vmArea == NULL || (flags & CACHE_DONT_LOCK_KERNEL_SPACE) != 0) { // This is either early in the boot process or we aren't allowed to // delete the area now. - _push(sFreeAreas, area); - sFreeAreaCount++; + _PushFreeArea(area); _RequestMaintenance(); return; } diff --git a/src/system/kernel/slab/MemoryManager.h b/src/system/kernel/slab/MemoryManager.h index 0fe20cb18a..9af7c96d20 100644 --- a/src/system/kernel/slab/MemoryManager.h +++ b/src/system/kernel/slab/MemoryManager.h @@ -15,6 +15,7 @@ #include #include "slab_debug.h" +#include "slab_private.h" class AbstractTraceEntryWithStackTrace; @@ -161,6 +162,9 @@ private: static void _PrepareMetaChunk(MetaChunk* metaChunk, size_t chunkSize); + static void _PushFreeArea(Area* area); + static Area* _PopFreeArea(); + static void _AddArea(Area* area); static status_t _AllocateArea(uint32 flags, Area*& _area); static void _FreeArea(Area* area, bool areaRemoved, @@ -235,6 +239,25 @@ MemoryManager::MaintenanceNeeded() } +/*static*/ inline void +MemoryManager::_PushFreeArea(Area* area) +{ + _push(sFreeAreas, area); + sFreeAreaCount++; +} + + +/*static*/ inline MemoryManager::Area* +MemoryManager::_PopFreeArea() +{ + if (sFreeAreaCount == 0) + return NULL; + + sFreeAreaCount--; + return _pop(sFreeAreas); +} + + /*static*/ inline addr_t MemoryManager::_AreaBaseAddressForAddress(addr_t address) {