From 49babc7caa82883fa891640406da89b68ae8d8e5 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sat, 16 Jul 2022 17:13:25 +1000 Subject: [PATCH] Cleanup: early exit MEM_lockfree_freeN when called with NULL pointer Simplify logic for freeing a NULL pointer. While no null-pointer de-reference was performed, this wasn't as so obvious as the pointer was passed to MEM_lockfree_allocN_len before checking for NULL. NOTE: T99744 claimed the a NULL pointer free was a vulnerability, while I can't see evidence for this - exiting early makes it clearer the memory isn't accessed. *Details* - Add MEMHEAD_LEN macro, avoids redundant NULL check. - Use "UNLIKELY(..)" hint's for error cases (freeing NULL pointer and checking if `leak_detector_has_run`). --- .../guardedalloc/intern/mallocn_lockfree_impl.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c index 73912ad07b1..300e2000a14 100644 --- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c +++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c @@ -44,6 +44,7 @@ enum { #define PTR_FROM_MEMHEAD(memhead) (memhead + 1) #define MEMHEAD_ALIGNED_FROM_PTR(ptr) (((MemHeadAligned *)ptr) - 1) #define MEMHEAD_IS_ALIGNED(memhead) ((memhead)->len & (size_t)MEMHEAD_ALIGN_FLAG) +#define MEMHEAD_LEN(memhead) ((memhead)->len & ~((size_t)(MEMHEAD_ALIGN_FLAG))) /* Uncomment this to have proper peak counter. */ #define USE_ATOMIC_MAX @@ -78,8 +79,8 @@ print_error(const char *str, ...) size_t MEM_lockfree_allocN_len(const void *vmemh) { - if (vmemh) { - return MEMHEAD_FROM_PTR(vmemh)->len & ~((size_t)(MEMHEAD_ALIGN_FLAG)); + if (LIKELY(vmemh)) { + return MEMHEAD_LEN(MEMHEAD_FROM_PTR(vmemh)); } return 0; @@ -87,14 +88,11 @@ size_t MEM_lockfree_allocN_len(const void *vmemh) void MEM_lockfree_freeN(void *vmemh) { - if (leak_detector_has_run) { + if (UNLIKELY(leak_detector_has_run)) { print_error("%s\n", free_after_leak_detection_message); } - MemHead *memh = MEMHEAD_FROM_PTR(vmemh); - size_t len = MEM_lockfree_allocN_len(vmemh); - - if (vmemh == NULL) { + if (UNLIKELY(vmemh == NULL)) { print_error("Attempt to free NULL pointer\n"); #ifdef WITH_ASSERT_ABORT abort(); @@ -102,6 +100,9 @@ void MEM_lockfree_freeN(void *vmemh) return; } + MemHead *memh = MEMHEAD_FROM_PTR(vmemh); + size_t len = MEMHEAD_LEN(memh); + atomic_sub_and_fetch_u(&totblock, 1); atomic_sub_and_fetch_z(&mem_in_use, len);