From 31c77baba054f44ce0ad12958b4096643b7fbb99 Mon Sep 17 00:00:00 2001 From: Hardening Date: Wed, 9 Apr 2014 21:53:42 +0200 Subject: [PATCH] Don't leak memory when realloc fails --- winpr/include/winpr/collections.h | 8 +-- winpr/libwinpr/utils/collections/ArrayList.c | 53 ++++++++++++----- winpr/libwinpr/utils/collections/BufferPool.c | 82 +++++++++++++++++++++------ 3 files changed, 110 insertions(+), 33 deletions(-) diff --git a/winpr/include/winpr/collections.h b/winpr/include/winpr/collections.h index 86bd34a..591e480 100644 --- a/winpr/include/winpr/collections.h +++ b/winpr/include/winpr/collections.h @@ -154,10 +154,10 @@ WINPR_API void ArrayList_Clear(wArrayList* arrayList); WINPR_API BOOL ArrayList_Contains(wArrayList* arrayList, void* obj); WINPR_API int ArrayList_Add(wArrayList* arrayList, void* obj); -WINPR_API void ArrayList_Insert(wArrayList* arrayList, int index, void* obj); +WINPR_API BOOL ArrayList_Insert(wArrayList* arrayList, int index, void* obj); -WINPR_API void ArrayList_Remove(wArrayList* arrayList, void* obj); -WINPR_API void ArrayList_RemoveAt(wArrayList* arrayList, int index); +WINPR_API BOOL ArrayList_Remove(wArrayList* arrayList, void* obj); +WINPR_API BOOL ArrayList_RemoveAt(wArrayList* arrayList, int index); WINPR_API int ArrayList_IndexOf(wArrayList* arrayList, void* obj, int startIndex, int count); WINPR_API int ArrayList_LastIndexOf(wArrayList* arrayList, void* obj, int startIndex, int count); @@ -387,7 +387,7 @@ WINPR_API int BufferPool_GetPoolSize(wBufferPool* pool); WINPR_API int BufferPool_GetBufferSize(wBufferPool* pool, void* buffer); WINPR_API void* BufferPool_Take(wBufferPool* pool, int bufferSize); -WINPR_API void BufferPool_Return(wBufferPool* pool, void* buffer); +WINPR_API BOOL BufferPool_Return(wBufferPool* pool, void* buffer); WINPR_API void BufferPool_Clear(wBufferPool* pool); WINPR_API wBufferPool* BufferPool_New(BOOL synchronized, int fixedSize, DWORD alignment); diff --git a/winpr/libwinpr/utils/collections/ArrayList.c b/winpr/libwinpr/utils/collections/ArrayList.c index 9ace8d5..938a17f 100644 --- a/winpr/libwinpr/utils/collections/ArrayList.c +++ b/winpr/libwinpr/utils/collections/ArrayList.c @@ -133,14 +133,20 @@ void ArrayList_SetItem(wArrayList* arrayList, int index, void* obj) * Shift a section of the list. */ -void ArrayList_Shift(wArrayList* arrayList, int index, int count) +BOOL ArrayList_Shift(wArrayList* arrayList, int index, int count) { if (count > 0) { if (arrayList->size + count > arrayList->capacity) { - arrayList->capacity *= arrayList->growthFactor; - arrayList->array = (void**) realloc(arrayList->array, sizeof(void*) * arrayList->capacity); + void **newArray; + int newCapacity = arrayList->capacity * arrayList->growthFactor; + + newArray = (void **)realloc(arrayList->array, sizeof(void*) * newCapacity); + if (!newArray) + return FALSE; + arrayList->array = newArray; + arrayList->capacity = newCapacity; } MoveMemory(&arrayList->array[index + count], &arrayList->array[index], (arrayList->size - index) * sizeof(void*)); @@ -153,6 +159,7 @@ void ArrayList_Shift(wArrayList* arrayList, int index, int count) MoveMemory(&arrayList->array[index], &arrayList->array[index - count], chunk * sizeof(void*)); arrayList->size += count; } + return TRUE; } /** @@ -201,20 +208,27 @@ BOOL ArrayList_Contains(wArrayList* arrayList, void* obj) int ArrayList_Add(wArrayList* arrayList, void* obj) { - int index; + int index = -1; if (arrayList->synchronized) EnterCriticalSection(&arrayList->lock); if (arrayList->size + 1 > arrayList->capacity) { - arrayList->capacity *= arrayList->growthFactor; - arrayList->array = (void**) realloc(arrayList->array, sizeof(void*) * arrayList->capacity); + void **newArray; + int newCapacity = arrayList->capacity * arrayList->growthFactor; + newArray = (void **)realloc(arrayList->array, sizeof(void*) * newCapacity); + if (!newArray) + goto out; + + arrayList->array = newArray; + arrayList->capacity = newCapacity; } arrayList->array[arrayList->size++] = obj; index = arrayList->size; +out: if (arrayList->synchronized) LeaveCriticalSection(&arrayList->lock); @@ -225,29 +239,38 @@ int ArrayList_Add(wArrayList* arrayList, void* obj) * Inserts an element into the ArrayList at the specified index. */ -void ArrayList_Insert(wArrayList* arrayList, int index, void* obj) +BOOL ArrayList_Insert(wArrayList* arrayList, int index, void* obj) { + BOOL ret = TRUE; if (arrayList->synchronized) EnterCriticalSection(&arrayList->lock); if ((index >= 0) && (index < arrayList->size)) { - ArrayList_Shift(arrayList, index, 1); - arrayList->array[index] = obj; + if (!ArrayList_Shift(arrayList, index, 1)) + { + ret = FALSE; + } + else + { + arrayList->array[index] = obj; + } } if (arrayList->synchronized) LeaveCriticalSection(&arrayList->lock); + return ret; } /** * Removes the first occurrence of a specific object from the ArrayList. */ -void ArrayList_Remove(wArrayList* arrayList, void* obj) +BOOL ArrayList_Remove(wArrayList* arrayList, void* obj) { int index; BOOL found = FALSE; + BOOL ret = TRUE; if (arrayList->synchronized) EnterCriticalSection(&arrayList->lock); @@ -262,28 +285,32 @@ void ArrayList_Remove(wArrayList* arrayList, void* obj) } if (found) - ArrayList_Shift(arrayList, index, -1); + ret = ArrayList_Shift(arrayList, index, -1); if (arrayList->synchronized) LeaveCriticalSection(&arrayList->lock); + return ret; } /** * Removes the element at the specified index of the ArrayList. */ -void ArrayList_RemoveAt(wArrayList* arrayList, int index) +BOOL ArrayList_RemoveAt(wArrayList* arrayList, int index) { + BOOL ret = TRUE; + if (arrayList->synchronized) EnterCriticalSection(&arrayList->lock); if ((index >= 0) && (index < arrayList->size)) { - ArrayList_Shift(arrayList, index, -1); + ret = ArrayList_Shift(arrayList, index, -1); } if (arrayList->synchronized) LeaveCriticalSection(&arrayList->lock); + return ret; } /** diff --git a/winpr/libwinpr/utils/collections/BufferPool.c b/winpr/libwinpr/utils/collections/BufferPool.c index 7dfae8e..885064b 100644 --- a/winpr/libwinpr/utils/collections/BufferPool.c +++ b/winpr/libwinpr/utils/collections/BufferPool.c @@ -34,14 +34,20 @@ * Methods */ -void BufferPool_ShiftAvailable(wBufferPool* pool, int index, int count) +BOOL BufferPool_ShiftAvailable(wBufferPool* pool, int index, int count) { if (count > 0) { if (pool->aSize + count > pool->aCapacity) { - pool->aCapacity *= 2; - pool->aArray = (wBufferPoolItem*) realloc(pool->aArray, sizeof(wBufferPoolItem) * pool->aCapacity); + wBufferPoolItem *newArray; + int newCapacity = pool->aCapacity * 2; + + newArray = (wBufferPoolItem*) realloc(pool->aArray, sizeof(wBufferPoolItem) * newCapacity); + if (!newArray) + return FALSE; + pool->aArray = newArray; + pool->aCapacity = newCapacity; } MoveMemory(&pool->aArray[index + count], &pool->aArray[index], (pool->aSize - index) * sizeof(wBufferPoolItem)); @@ -52,16 +58,21 @@ void BufferPool_ShiftAvailable(wBufferPool* pool, int index, int count) MoveMemory(&pool->aArray[index], &pool->aArray[index - count], (pool->aSize - index) * sizeof(wBufferPoolItem)); pool->aSize += count; } + return TRUE; } -void BufferPool_ShiftUsed(wBufferPool* pool, int index, int count) +BOOL BufferPool_ShiftUsed(wBufferPool* pool, int index, int count) { if (count > 0) { if (pool->uSize + count > pool->uCapacity) { - pool->uCapacity *= 2; - pool->uArray = (wBufferPoolItem*) realloc(pool->uArray, sizeof(wBufferPoolItem) * pool->uCapacity); + int newUCapacity = pool->uCapacity * 2; + wBufferPoolItem *newUArray = (wBufferPoolItem *)realloc(pool->uArray, sizeof(wBufferPoolItem) *newUCapacity); + if (!newUArray) + return FALSE; + pool->uCapacity = newUCapacity; + pool->uArray = newUArray; } MoveMemory(&pool->uArray[index + count], &pool->uArray[index], (pool->uSize - index) * sizeof(wBufferPoolItem)); @@ -72,6 +83,7 @@ void BufferPool_ShiftUsed(wBufferPool* pool, int index, int count) MoveMemory(&pool->uArray[index], &pool->uArray[index - count], (pool->uSize - index) * sizeof(wBufferPoolItem)); pool->uSize += count; } + return TRUE; } /** @@ -172,6 +184,9 @@ void* BufferPool_Take(wBufferPool* pool, int size) else buffer = malloc(pool->fixedSize); } + + if (!buffer) + goto out_error; } else { @@ -225,17 +240,30 @@ void* BufferPool_Take(wBufferPool* pool, int size) } else { - buffer = realloc(buffer, size); + void *newBuffer = realloc(buffer, size); + if (!newBuffer) + goto out_error; + + buffer = newBuffer; } } - BufferPool_ShiftAvailable(pool, foundIndex, -1); + if (!BufferPool_ShiftAvailable(pool, foundIndex, -1)) + goto out_error; } + if (!buffer) + goto out_error; + if (pool->uSize + 1 > pool->uCapacity) { - pool->uCapacity *= 2; - pool->uArray = (wBufferPoolItem*) realloc(pool->uArray, sizeof(wBufferPoolItem) * pool->uCapacity); + int newUCapacity = pool->uCapacity * 2; + wBufferPoolItem *newUArray = (wBufferPoolItem *)realloc(pool->uArray, sizeof(wBufferPoolItem) * newUCapacity); + if (!newUArray) + goto out_error; + + pool->uCapacity = newUCapacity; + pool->uArray = newUArray; } pool->uArray[pool->uSize].buffer = buffer; @@ -247,13 +275,18 @@ void* BufferPool_Take(wBufferPool* pool, int size) LeaveCriticalSection(&pool->lock); return buffer; + +out_error: + if (pool->synchronized) + LeaveCriticalSection(&pool->lock); + return NULL; } /** * Returns a buffer to the pool. */ -void BufferPool_Return(wBufferPool* pool, void* buffer) +BOOL BufferPool_Return(wBufferPool* pool, void* buffer) { int size = 0; int index = 0; @@ -268,8 +301,13 @@ void BufferPool_Return(wBufferPool* pool, void* buffer) if ((pool->size + 1) >= pool->capacity) { - pool->capacity *= 2; - pool->array = (void**) realloc(pool->array, sizeof(void*) * pool->capacity); + int newCapacity = pool->capacity * 2; + void **newArray = (void **)realloc(pool->array, sizeof(void*) * newCapacity); + if (!newArray) + goto out_error; + + pool->capacity = newCapacity; + pool->array = newArray; } pool->array[(pool->size)++] = buffer; @@ -290,15 +328,21 @@ void BufferPool_Return(wBufferPool* pool, void* buffer) if (found) { size = pool->uArray[index].size; - BufferPool_ShiftUsed(pool, index, -1); + if (!BufferPool_ShiftUsed(pool, index, -1)) + goto out_error; } if (size) { if ((pool->aSize + 1) >= pool->aCapacity) { - pool->aCapacity *= 2; - pool->aArray = (wBufferPoolItem*) realloc(pool->aArray, sizeof(wBufferPoolItem) * pool->aCapacity); + int newCapacity = pool->aCapacity * 2; + wBufferPoolItem *newArray = (wBufferPoolItem*) realloc(pool->aArray, sizeof(wBufferPoolItem) * newCapacity); + if (!newArray) + goto out_error; + + pool->aCapacity = newCapacity; + pool->aArray = newArray; } pool->aArray[pool->aSize].buffer = buffer; @@ -309,6 +353,12 @@ void BufferPool_Return(wBufferPool* pool, void* buffer) if (pool->synchronized) LeaveCriticalSection(&pool->lock); + return TRUE; + +out_error: + if (pool->synchronized) + LeaveCriticalSection(&pool->lock); + return FALSE; } /** -- 2.7.4