Align values up safely

Co-authored-by: [email protected]

Author: PatKamin

src/base_alloc/base_alloc.c

@@ -134,7 +134,10 @@ static void *ba_os_alloc_annotated(size_t pool_size) {
 }
 
 umf_ba_pool_t *umf_ba_create(size_t size) {
-    size_t chunk_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
+    size_t chunk_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
+    if (chunk_size == 0) {
+        return NULL;
+    }
     size_t mutex_size = ALIGN_UP(utils_mutex_get_size(), MEMORY_ALIGNMENT);
 
     size_t metadata_size = sizeof(struct umf_ba_main_pool_meta_t);
@@ -144,7 +147,10 @@ umf_ba_pool_t *umf_ba_create(size_t size) {
         pool_size = MINIMUM_POOL_SIZE;
     }
 
-    pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
+    pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
+    if (pool_size == 0) {
+        return NULL;
+    }
 
     umf_ba_pool_t *pool = (umf_ba_pool_t *)ba_os_alloc_annotated(pool_size);
     if (!pool) {

src/base_alloc/base_alloc_global.c

@@ -96,8 +96,12 @@ static void *add_metadata_and_align(void *ptr, size_t size, size_t alignment) {
     if (alignment <= ALLOC_METADATA_SIZE) {
         user_ptr = (void *)((uintptr_t)ptr + ALLOC_METADATA_SIZE);
     } else {
-        user_ptr =
-            (void *)ALIGN_UP((uintptr_t)ptr + ALLOC_METADATA_SIZE, alignment);
+        user_ptr = (void *)ALIGN_UP_SAFE((uintptr_t)ptr + ALLOC_METADATA_SIZE,
+                                         alignment);
+        if (!user_ptr) {
+            LOG_ERR("base_alloc: pointer alignment overflow");
+            return NULL;
+        }
     }
 
     size_t ptr_offset_from_original = (uintptr_t)user_ptr - (uintptr_t)ptr;

src/base_alloc/base_alloc_linear.c

@@ -88,7 +88,11 @@ umf_ba_linear_pool_t *umf_ba_linear_create(size_t pool_size) {
         pool_size = MINIMUM_LINEAR_POOL_SIZE;
     }
 
-    pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
+    pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
+    if (pool_size == 0) {
+        LOG_ERR("pool_size page alignment overflow");
+        return NULL;
+    }
 
     umf_ba_linear_pool_t *pool = (umf_ba_linear_pool_t *)ba_os_alloc(pool_size);
     if (!pool) {
@@ -122,15 +126,24 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) {
     if (size == 0) {
         return NULL;
     }
-    size_t aligned_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
+    size_t aligned_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
+    if (aligned_size == 0) {
+        LOG_ERR("size alignment overflow");
+        return NULL;
+    }
     utils_mutex_lock(&pool->metadata.lock);
     if (pool->metadata.size_left < aligned_size) {
         size_t pool_size = MINIMUM_LINEAR_POOL_SIZE;
         size_t usable_size =
             pool_size - offsetof(umf_ba_next_linear_pool_t, data);
         if (usable_size < aligned_size) {
             pool_size += aligned_size - usable_size;
-            pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
+            pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
+            if (pool_size == 0) {
+                utils_mutex_unlock(&pool->metadata.lock);
+                LOG_ERR("pool_size page alignment overflow");
+                return NULL;
+            }
         }
 
         assert(pool_size - offsetof(umf_ba_next_linear_pool_t, data) >=

src/provider/provider_os_memory.c

@@ -834,12 +834,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
     membind_t membind;
     memset(&membind, 0, sizeof(membind));
 
-    membind.alloc_size = ALIGN_UP(size, page_size);
+    membind.alloc_size = size;
     membind.page_size = page_size;
     membind.addr = addr;
     membind.pages = membind.alloc_size / membind.page_size;
     if (provider->nodeset_len == 1) {
-        membind.bind_size = ALIGN_UP(size, membind.page_size);
+        membind.bind_size = size;
         membind.bitmap = provider->nodeset[0];
         return membind;
     }
@@ -945,7 +945,15 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
 
     // Bind memory to NUMA nodes if numa_policy is other than DEFAULT
     if (os_provider->numa_policy != HWLOC_MEMBIND_DEFAULT) {
-        membind_t membind = membindFirst(os_provider, addr, size, page_size);
+        size_t first_size = ALIGN_UP_SAFE(size, page_size);
+        if (first_size == 0) {
+            LOG_ERR("size is too big, page align failed");
+            (void)utils_munmap(addr, size);
+            return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+        }
+
+        membind_t membind =
+            membindFirst(os_provider, addr, first_size, page_size);
         if (membind.bitmap == NULL) {
             goto err_unmap;
         }

src/proxy_lib/proxy_lib.c

@@ -231,7 +231,7 @@ static inline void *ba_leak_realloc(void *ptr, size_t size, size_t max_size) {
 static inline void *ba_leak_aligned_alloc(size_t alignment, size_t size) {
     ba_leak_init_once();
     void *ptr = umf_ba_linear_alloc(Base_alloc_leak, size + alignment);
-    return (void *)ALIGN_UP((uintptr_t)ptr, alignment);
+    return (void *)ALIGN_UP_SAFE((uintptr_t)ptr, alignment);
 }
 
 static inline int ba_leak_free(void *ptr) {

src/utils/utils_common.h

@@ -40,6 +40,10 @@ typedef enum umf_purge_advise_t {
 #define IS_NOT_ALIGNED(value, align)                                           \
     ((align != 0 && (((value) & ((align)-1)) != 0)))
 #define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
+#define ALIGN_UP_SAFE(value, align)                                            \
+    (((align) == 0)                                                            \
+         ? (value)                                                             \
+         : (((value) + (align)-1) < (value) ? 0 : ALIGN_UP((value), (align))))
 #define ALIGN_DOWN(value, align) ((value) & ~((align)-1))
 #define ASSERT_IS_ALIGNED(value, align)                                        \
     DO_WHILE_EXPRS(assert(IS_ALIGNED(value, align)))

test/common/provider.hpp

@@ -105,15 +105,14 @@ struct provider_ba_global : public provider_base_t {
             align = 8;
         }
 
-        if (size > SIZE_MAX - align + 1) {
-            return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
-        }
-
         // aligned_malloc returns a valid pointer despite not meeting the
         // requirement of 'size' being multiple of 'align' even though the
         // documentation says that it has to. AddressSanitizer returns an
         // error because of this issue.
-        size_t aligned_size = ALIGN_UP(size, align);
+        size_t aligned_size = ALIGN_UP_SAFE(size, align);
+        if (aligned_size == 0) {
+            return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
+        }
 
         *ptr = umf_ba_global_aligned_alloc(aligned_size, align);
 

test/common/test_helpers.h

@@ -14,6 +14,7 @@
 #include <umf/memory_provider_ops.h>
 
 #include "provider_trace.h"
+#include "utils_common.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -22,10 +23,6 @@ extern "C" {
 // Needed for CI
 #define TEST_SKIP_ERROR_CODE 125
 
-#ifndef ALIGN_UP
-#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
-#endif
-
 int bufferIsFilledWithChar(void *ptr, size_t size, char c);
 
 int buffersHaveSameContent(void *first, void *second, size_t size);

test/supp/helgrind-umf_test-disjointPool.supp

@@ -27,3 +27,27 @@
    fun:*gthread_mutex_unlock*pthread_mutex_t
    ...
 }
+
+{
+   Incompatibility with helgrind's implementation ("pthread_rwlock_{rd,rw}lock with a pthread_mutex_t* argument")
+   Helgrind:Misc
+   obj:*vgpreload_helgrind-amd64-linux.so
+   fun:*glibcxx_rwlock_wrlock*pthread_rwlock_t
+   ...
+}
+
+{
+   Incompatibility with helgrind's implementation ("pthread_rwlock_unlock with a pthread_mutex_t* argument")
+   Helgrind:Misc
+   obj:*vgpreload_helgrind-amd64-linux.so
+   fun:*glibcxx_rwlock_unlock*pthread_rwlock_t
+   ...
+}
+
+{
+   Incompatibility with helgrind's implementation ("pthread_rwlock_{rd,rw}lock with a pthread_mutex_t* argument")
+   Helgrind:Misc
+   obj:*vgpreload_helgrind-amd64-linux.so
+   fun:*glibcxx_rwlock_rdlock*pthread_rwlock_t*
+   ...
+}