From faaa12c51f442b1705ffacf1823dad54aa4d9e44 Mon Sep 17 00:00:00 2001 From: Jordan Bancino Date: Mon, 29 May 2023 23:53:17 +0000 Subject: [PATCH] Re-add memory bounds checking. Also fixed a recursive lock error in some configurations, and replaced a usage of strcpy() with strncpy(). --- Cytoplasm/src/Memory.c | 77 ++++++++++++++++++++++++++++++------------ Cytoplasm/src/Util.c | 4 +-- src/Config.c | 2 +- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/Cytoplasm/src/Memory.c b/Cytoplasm/src/Memory.c index aa6dacb..99fcb25 100644 --- a/Cytoplasm/src/Memory.c +++ b/Cytoplasm/src/Memory.c @@ -50,6 +50,13 @@ struct MemoryInfo void *pointer; }; +#define MEM_BOUND_TYPE UInt32 +#define MEM_BOUND 0xDEADBEEF + +#define MEM_BOUND_LOWER(p) *((MEM_BOUND_TYPE *) p) +#define MEM_BOUND_UPPER(p, x) *(((MEM_BOUND_TYPE *) (((UInt8 *) p) + x)) + 1) +#define MEM_SIZE_ACTUAL(x) (((x) * sizeof(UInt8)) + (2 * sizeof(MEM_BOUND_TYPE))) + static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; static void (*hook) (MemoryAction, MemoryInfo *, void *) = NULL; static void *hookArgs = NULL; @@ -149,12 +156,29 @@ MemoryDelete(MemoryInfo * a) } } +static int +MemoryCheck(MemoryInfo * a) +{ + if (MEM_BOUND_LOWER(a->pointer) != MEM_BOUND || + MEM_BOUND_UPPER(a->pointer, a->size - (2 * sizeof(MEM_BOUND_TYPE))) != MEM_BOUND) + { + if (hook) + { + hook(MEMORY_CORRUPTED, a, hookArgs); + } + return 0; + } + return 1; +} + void * MemoryAllocate(size_t size, const char *file, int line) { void *p; MemoryInfo *a; + MemoryIterate(NULL, NULL); + pthread_mutex_lock(&lock); a = malloc(sizeof(MemoryInfo)); @@ -164,7 +188,7 @@ MemoryAllocate(size_t size, const char *file, int line) return NULL; } - p = malloc(size); + p = malloc(MEM_SIZE_ACTUAL(size)); if (!p) { free(a); @@ -172,9 +196,11 @@ MemoryAllocate(size_t size, const char *file, int line) return NULL; } - memset(p, 0, size); + memset(p, 0, MEM_SIZE_ACTUAL(size)); + MEM_BOUND_LOWER(p) = MEM_BOUND; + MEM_BOUND_UPPER(p, size) = MEM_BOUND; - a->size = size; + a->size = MEM_SIZE_ACTUAL(size); a->file = file; a->line = line; a->pointer = p; @@ -193,7 +219,7 @@ MemoryAllocate(size_t size, const char *file, int line) } pthread_mutex_unlock(&lock); - return p; + return ((MEM_BOUND_TYPE *) p) + 1; } void * @@ -202,6 +228,8 @@ MemoryReallocate(void *p, size_t size, const char *file, int line) MemoryInfo *a; void *new = NULL; + MemoryIterate(NULL, NULL); + if (!p) { return MemoryAllocate(size, file, line); @@ -211,17 +239,20 @@ MemoryReallocate(void *p, size_t size, const char *file, int line) if (a) { pthread_mutex_lock(&lock); - new = realloc(a->pointer, size); + new = realloc(a->pointer, MEM_SIZE_ACTUAL(size)); if (new) { MemoryDelete(a); - a->size = size; + a->size = MEM_SIZE_ACTUAL(size); a->file = file; a->line = line; a->pointer = new; MemoryInsert(a); + MEM_BOUND_LOWER(a->pointer) = MEM_BOUND; + MEM_BOUND_UPPER(a->pointer, size) = MEM_BOUND; + if (hook) { hook(MEMORY_REALLOCATE, a, hookArgs); @@ -244,7 +275,7 @@ MemoryReallocate(void *p, size_t size, const char *file, int line) } } - return new; + return ((MEM_BOUND_TYPE *) new) + 1; } void @@ -252,6 +283,8 @@ MemoryFree(void *p, const char *file, int line) { MemoryInfo *a; + MemoryIterate(NULL, NULL); + if (!p) { return; @@ -340,6 +373,7 @@ MemoryInfoGet(void *p) pthread_mutex_lock(&lock); + p = ((MEM_BOUND_TYPE *) p) - 1; hash = MemoryHash(p); count = 0; @@ -352,6 +386,7 @@ MemoryInfoGet(void *p) } else { + MemoryCheck(allocations[hash]); pthread_mutex_unlock(&lock); return allocations[hash]; } @@ -369,7 +404,7 @@ MemoryInfoGetSize(MemoryInfo * a) return 0; } - return a->size; + return a->size ? a->size - (2 * sizeof(MEM_BOUND_TYPE)) : 0; } const char * @@ -402,7 +437,7 @@ MemoryInfoGetPointer(MemoryInfo * a) return NULL; } - return a->pointer; + return ((MEM_BOUND_TYPE *) a->pointer) + 1; } void @@ -416,6 +451,7 @@ void { if (allocations[i]) { + MemoryCheck(allocations[i]); if (iterFunc) { iterFunc(allocations[i], args); @@ -471,28 +507,27 @@ MemoryDefaultHook(MemoryAction a, MemoryInfo * i, void *args) switch (a) { case MEMORY_BAD_POINTER: - write(STDOUT_FILENO, "Bad pointer: 0x", 15); + write(STDERR_FILENO, "Bad pointer: 0x", 15); break; case MEMORY_CORRUPTED: - write(STDOUT_FILENO, "Corrupted block: 0x", 19); + write(STDERR_FILENO, "Corrupted block: 0x", 19); break; default: return; } - write(STDOUT_FILENO, buf, len); - write(STDOUT_FILENO, " to 0x", 6); + write(STDERR_FILENO, buf, len); + write(STDERR_FILENO, " to 0x", 6); len = HexPtr(MemoryInfoGetSize(i), buf, sizeof(buf)); - write(STDOUT_FILENO, buf, len); - write(STDOUT_FILENO, " bytes at ", 10); - write(STDOUT_FILENO, MemoryInfoGetFile(i), strlen(MemoryInfoGetFile(i))); - write(STDOUT_FILENO, ":0x", 3); + write(STDERR_FILENO, buf, len); + write(STDERR_FILENO, " bytes at ", 10); + write(STDERR_FILENO, MemoryInfoGetFile(i), strlen(MemoryInfoGetFile(i))); + write(STDERR_FILENO, ":0x", 3); len = HexPtr(MemoryInfoGetLine(i), buf, sizeof(buf)); - write(STDOUT_FILENO, buf, len); - write(STDOUT_FILENO, "\n", 1); -#if 0 + write(STDERR_FILENO, buf, len); + write(STDERR_FILENO, "\n", 1); + raise(SIGSEGV); -#endif } void diff --git a/Cytoplasm/src/Util.c b/Cytoplasm/src/Util.c index 78c220a..6a274c3 100644 --- a/Cytoplasm/src/Util.c +++ b/Cytoplasm/src/Util.c @@ -248,7 +248,7 @@ UtilGetLine(char **linePtr, size_t * n, Stream * stream) static void ThreadNoDestructor(void *p) { - Free(p); + free(p); } unsigned long @@ -269,7 +269,7 @@ UtilThreadNo(void) no = pthread_getspecific(key); if (!no) { - no = Malloc(sizeof(unsigned long)); + no = malloc(sizeof(unsigned long)); *no = count++; pthread_setspecific(key, no); } diff --git a/src/Config.c b/src/Config.c index ac1aaa5..475aaba 100644 --- a/src/Config.c +++ b/src/Config.c @@ -457,7 +457,7 @@ ConfigCreateDefault(Db * db) if (gethostname(hostname, HOST_NAME_MAX + 1) < 0) { - strcpy(hostname, "localhost"); + strncpy(hostname, "localhost", HOST_NAME_MAX); } HashMapSet(json, "serverName", JsonValueString(hostname));