From: Urban Wallasch Date: Sun, 10 Nov 2019 14:52:41 +0000 (+0100) Subject: * tmalloc: fixed memory leak (the irony!); refactoring; small improvements. X-Git-Url: https://git.packet-gain.de/?a=commitdiff_plain;h=b3637c47ba896dce02de583d4f2738952718a21b;p=oddbits.git * tmalloc: fixed memory leak (the irony!); refactoring; small improvements. --- diff --git a/tmalloc/tmalloc.c b/tmalloc/tmalloc.c index 8d4bdd9..8269b5c 100644 --- a/tmalloc/tmalloc.c +++ b/tmalloc/tmalloc.c @@ -94,7 +94,7 @@ static inline void log_hex(uint64_t n) { write(tmalloc_log_fd, b, sizeof buf - (b - buf)); } -static inline void log_xdump(const void *s, size_t n) { +static inline void log_xdump(const char *pfx, const void *s, size_t n) { const unsigned char *p = s; char buf[68]; char *b = buf; @@ -109,6 +109,7 @@ static inline void log_xdump(const void *s, size_t n) { ++i; if (i % 16 == 0) { *a++ = '\n'; + log_str(pfx); write(tmalloc_log_fd, buf, a - buf); memset(buf, ' ', sizeof buf); b = buf; @@ -122,6 +123,7 @@ static inline void log_xdump(const void *s, size_t n) { } if (b != buf) { *a++ = '\n'; + log_str(pfx); write(tmalloc_log_fd, buf, a - buf); } } @@ -147,7 +149,7 @@ static inline void tm_panic(void) { * Hash table to store information about allocated blocks of memory. */ -#define TM_HASHTAB_SIZE 1024 +#define TM_HASHTAB_SIZE 2048 struct tm_info_struct { void *addr; @@ -182,34 +184,14 @@ static inline uint32_t tm_hash(uint64_t k) { return hash % TM_HASHTAB_SIZE; } -static inline int tm_ht_insert(void *addr, size_t size, - void *raddr, size_t rsize, void *caller) { - struct tm_info_struct *e = __libc_malloc(sizeof *e); - uint32_t h; +static inline struct tm_info_struct *tm_ht_lookup(void *addr) { + uint32_t h = tm_hash((uint64_t)addr); - if (!e) { - log_str("ERROR: failed to allocate internal block info struct"); - tm_panic(); - } - h = tm_hash((uint64_t)addr); for (struct tm_info_struct *p = tm_htab[h]; p; p = p->next) { - if (p->addr == addr) { - log_str("ERROR: block at"); - log_hex((uint64_t)p->addr); log_str(" ("); - log_hex((uint64_t)p->raddr); log_str(") "); - log_str("already listed as allocated"); - tm_panic(); - return -1; - } + if (p->addr == addr) + return p; } - e->addr = addr; - e->size = size; - e->raddr = raddr; - e->rsize = rsize; - e->caller = caller; - e->next = tm_htab[h]; - tm_htab[h] = e; - return 0; + return NULL; } static inline int tm_ht_check_entry(struct tm_info_struct *p) { @@ -240,26 +222,57 @@ static inline int tm_ht_check_entry(struct tm_info_struct *p) { return res; } -static inline int tm_ht_update(void *addr, size_t size, +static inline int tm_ht_insert(void *addr, size_t size, void *raddr, size_t rsize, void *caller) { struct tm_info_struct *p; uint32_t h; + p = tm_ht_lookup(addr); + if (p) { + log_str("ERROR: block at"); + log_hex((uint64_t)p->addr); log_str(" ("); + log_hex((uint64_t)p->raddr); log_str(") "); + log_str("already listed as allocated"); + tm_panic(); + return -1; + } + p = __libc_malloc(sizeof *p); + if (!p) { + log_str("ERROR: failed to allocate internal block info struct"); + tm_panic(); + return -1; + } + memcpy(raddr, &tm_sentinel, TM_SENTSZ); + memcpy(raddr + rsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ); h = tm_hash((uint64_t)addr); - for (p = tm_htab[h]; p; p = p->next) { - if (p->addr == addr) { - p->size = size; - p->raddr = raddr; - p->rsize = rsize; - p->caller = caller; - return 0; - } + p->addr = addr; + p->size = size; + p->raddr = raddr; + p->rsize = rsize; + p->caller = caller; + p->next = tm_htab[h]; + tm_htab[h] = p; + return 0; +} + +static inline int tm_ht_update(void *addr, size_t size, + void *raddr, size_t rsize, void *caller) { + struct tm_info_struct *p; + + p = tm_ht_lookup(addr); + if (!p) { + log_str(TM_LOG_PFX"ERROR: attempt to update block at "); + log_hex((uint64_t)addr); + log_str(", not listed as allocated!\n"); + tm_panic(); + return -1; } - log_str(TM_LOG_PFX"ERROR: attempt to update block at "); - log_hex((uint64_t)addr); - log_str(", not listed as allocated!\n"); - tm_panic(); - return -1; + memcpy(raddr + rsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ); + p->size = size; + p->raddr = raddr; + p->rsize = rsize; + p->caller = caller; + return 0; } static inline int tm_ht_remove(void *addr, int wipe) { @@ -269,12 +282,16 @@ static inline int tm_ht_remove(void *addr, int wipe) { h = tm_hash((uint64_t)addr); for (p = tm_htab[h]; p; p = p->next) { if (p->addr == addr) { - if (prev == NULL) + if (!prev) tm_htab[h] = p->next; else prev->next = p->next; - if (wipe) + if (wipe) { + /* NOTE: Only check when wiping, else there be dragons! */ + tm_ht_check_entry(p); memset(p->raddr, '\x55', p->rsize); + } + __libc_free(p); return 0; } prev = p; @@ -286,36 +303,18 @@ static inline int tm_ht_remove(void *addr, int wipe) { return -1; } -static inline int tm_ht_check(void *addr) { - struct tm_info_struct *p; - uint32_t h; - - h = tm_hash((uint64_t)addr); - for (p = tm_htab[h]; p; p = p->next) { - if (p->addr == addr) { - return tm_ht_check_entry(p); - } - } - log_str(TM_LOG_PFX"ERROR: attempt to check block at "); - log_hex((uint64_t)addr); - log_str(", not listed as allocated!\n"); - tm_panic(); - return -1; -} - static inline int tm_ht_checkall(void) { - size_t i, n; + size_t i; + int res = 0; struct tm_info_struct *p; - for (i = n = 0; i < TM_HASHTAB_SIZE; ++i) { - for (p = tm_htab[i]; p; p = p->next) { - if (tm_ht_check_entry(p) != 0) - ++n; - } + for (i = 0; i < TM_HASHTAB_SIZE; ++i) { + for (p = tm_htab[i]; p; p = p->next) + res -= tm_ht_check_entry(p); } log_str(TM_LOG_PFX"CHECK: "); - log_dec(n); log_str(" corrupted block(s) found.\n"); - return n ? -1 : 0; + log_dec(res); log_str(" corrupted block(s) found.\n"); + return res; } static inline void tm_ht_dump(void) { @@ -334,7 +333,7 @@ static inline void tm_ht_dump(void) { log_str("\n"); } if (tmalloc_log_level >= TM_LOG_DEBUG) { - log_xdump(p->addr, p->size > 64 ? 64 : p->size); + log_xdump(" ", p->addr, p->size > 64 ? 64 : p->size); } tm_ht_check_entry(p); ++n; @@ -355,7 +354,7 @@ static inline void tm_ht_dump(void) { #ifdef __GNUC__ #define ret_address(l_) __builtin_return_address(l_) #else - #define ret_address(l_) ((void *)0xdeadbeefcafebabeULL) + #define ret_address(l_) ((void *)0xdefecatedeadf00dULL) #endif void *malloc(size_t size) { @@ -363,18 +362,16 @@ void *malloc(size_t size) { void *addr; size_t xsize = size + (TM_SENTSZ * 2); - TM_HT_LOCK(); addr = __libc_malloc(xsize); if (addr) { - memcpy(addr, &tm_sentinel, TM_SENTSZ); - memcpy(addr + xsize - TM_SENTSZ, &tm_sentinel, TM_SENTSZ); + TM_HT_LOCK(); tm_ht_insert(addr + TM_SENTSZ, size, addr, xsize, caller); + TM_HT_UNLOCK(); addr += TM_SENTSZ; } else if (tmalloc_log_level >= TM_LOG_WARNING) { log_str(TM_LOG_PFX"WARNING: __libc_malloc() returned NULL pointer!\n"); } - TM_HT_UNLOCK(); if (tmalloc_log_level >= TM_LOG_TRACE) { log_str(TM_LOG_PFX"TRACE: malloc("); log_dec(size); @@ -390,18 +387,16 @@ void *calloc(size_t nmemb, size_t size) { void *addr; size_t xnmemb = nmemb + ((TM_SENTSZ - 1) / size + 1) * 2; - TM_HT_LOCK(); addr = __libc_calloc(xnmemb, size); if (addr) { - memcpy(addr, &tm_sentinel, TM_SENTSZ); - memcpy(addr + xnmemb * size - TM_SENTSZ, &tm_sentinel, sizeof tm_sentinel); + TM_HT_LOCK(); tm_ht_insert(addr + TM_SENTSZ, nmemb * size, addr, xnmemb * size, caller); + TM_HT_UNLOCK(); addr += TM_SENTSZ; } else if (tmalloc_log_level >= TM_LOG_WARNING) { log_str(TM_LOG_PFX"WARNING: __libc_calloc() returned NULL pointer!\n"); } - TM_HT_UNLOCK(); if (tmalloc_log_level >= TM_LOG_TRACE) { log_str(TM_LOG_PFX"TRACE: calloc("); @@ -419,25 +414,25 @@ void *realloc(void *ptr, size_t size) { void *oaddr = ptr ? ptr - TM_SENTSZ : NULL; size_t xsize = size + (TM_SENTSZ * 2); - TM_HT_LOCK(); addr = __libc_realloc(oaddr, xsize); if (addr) { + TM_HT_LOCK(); if (addr == oaddr) { tm_ht_update(ptr, size, addr, xsize, caller); } else { - if (ptr) + if (ptr) { + /* NOTE: Do not wipe, it's not ours anymore! */ tm_ht_remove(ptr, 0); + } tm_ht_insert(addr + TM_SENTSZ, size, addr, xsize, caller); - memcpy(addr, &tm_sentinel, TM_SENTSZ); } - memcpy(addr + xsize - TM_SENTSZ, &tm_sentinel, sizeof tm_sentinel); + TM_HT_UNLOCK(); addr += TM_SENTSZ; } else if (tmalloc_log_level >= TM_LOG_WARNING) { log_str(TM_LOG_PFX"WARNING: __libc_realloc() returned NULL pointer!\n"); } - TM_HT_UNLOCK(); if (tmalloc_log_level >= TM_LOG_TRACE) { log_str(TM_LOG_PFX"TRACE: realloc("); @@ -460,15 +455,14 @@ void free(void *ptr) { if (ptr) { TM_HT_LOCK(); - tm_ht_check(ptr); if (tm_ht_remove(ptr, tmalloc_wipe_on_free) != 0) { log_str(TM_LOG_PFX"ERROR: possible double free("); log_hex((uint64_t)ptr); log_str("): *** HEAP IS VERY LIKELY CORRUPT! ***\n"); tm_panic(); } - __libc_free(ptr - TM_SENTSZ); TM_HT_UNLOCK(); + __libc_free(ptr - TM_SENTSZ); } else if (tmalloc_log_level >= TM_LOG_INFO) { log_str(TM_LOG_PFX"INFO: free() called with NULL pointer argument!\n"); @@ -497,10 +491,13 @@ void tmalloc_set_wipe_on_free(int w) { tmalloc_wipe_on_free = w; } -void tmalloc_check(void) { +int tmalloc_check(void) { + int res = 0; + TM_HT_LOCK(); - tm_ht_checkall(); + res = tm_ht_checkall(); TM_HT_UNLOCK(); + return res; } void tmalloc_stat(void) { diff --git a/tmalloc/tmalloc.h b/tmalloc/tmalloc.h index 2c404c4..7dbef6c 100644 --- a/tmalloc/tmalloc.h +++ b/tmalloc/tmalloc.h @@ -54,8 +54,10 @@ void tmalloc_set_wipe_on_free(int w); /* * Check all known allocated blocks for intact sentinel values * and report any problems detected. + * + * Returns the number of corrupt blocks found. */ -extern void tmalloc_check(void); +extern int tmalloc_check(void); /* * Print an overview of all known allocated blocks.