Repository: oneapi-src/unified-memory-framework (language: python)

Issue:
Add tests for integer overflow
The application performs a calculation that can produce an integer overflow or underflow. This can happen when an application's logic assumes that a resulting value will always be larger or smaller than the original value. This can lead to other weaknesses because the size of the buffer is incorrectly calculated, such as a buffer overflow.

Use the following guidelines to test for integer overflow:
- Identify sections of the site that perform integer arithmetic with user-supplied values.
- Provide large negative and positive values and inspect the result.
- This test fails if the result is negative when it should be positive, or vice-versa.


Discussion / hints:
note: use "unsigned" where this make sense 

Fix patch:
diff --git a/.github/workflows/reusable_sanitizers.yml b/.github/workflows/reusable_sanitizers.yml
index 2c63ebd51..3acda6833 100644
--- a/.github/workflows/reusable_sanitizers.yml
+++ b/.github/workflows/reusable_sanitizers.yml
@@ -73,6 +73,9 @@ jobs:
 
     - name: Run tests
       working-directory: ${{env.BUILD_DIR}}
+      env:
+        ASAN_OPTIONS: allocator_may_return_null=1
+        TSAN_OPTIONS: allocator_may_return_null=1
       run: |
         ${{ matrix.compiler.cxx == 'icpx' && '. /opt/intel/oneapi/setvars.sh' || true }}
         ctest --output-on-failure
@@ -141,4 +144,7 @@ jobs:
 
     - name: Run tests
       working-directory: ${{env.BUILD_DIR}}
+      env:
+        ASAN_OPTIONS: allocator_may_return_null=1
+        TSAN_OPTIONS: allocator_may_return_null=1
       run: ctest -C Debug --output-on-failure
diff --git a/src/base_alloc/base_alloc.c b/src/base_alloc/base_alloc.c
index 7a98684c6..209ace7fe 100644
--- a/src/base_alloc/base_alloc.c
+++ b/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) {
diff --git a/src/base_alloc/base_alloc_global.c b/src/base_alloc/base_alloc_global.c
index ec6bc9fcb..11d88b731 100644
--- a/src/base_alloc/base_alloc_global.c
+++ b/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;
@@ -155,10 +159,19 @@ void *umf_ba_global_aligned_alloc(size_t size, size_t alignment) {
         return NULL;
     }
 
+    if (size > SIZE_MAX - ALLOC_METADATA_SIZE) {
+        LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
+        return NULL;
+    }
+
     // for metadata
     size += ALLOC_METADATA_SIZE;
 
     if (alignment > ALLOC_METADATA_SIZE) {
+        if (size > SIZE_MAX - alignment) {
+            LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
+            return NULL;
+        }
         size += alignment;
     }
 
diff --git a/src/base_alloc/base_alloc_linear.c b/src/base_alloc/base_alloc_linear.c
index 8773e5cab..a35a6c243 100644
--- a/src/base_alloc/base_alloc_linear.c
+++ b/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,7 +126,11 @@ 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;
@@ -130,7 +138,12 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t 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) >=
diff --git a/src/provider/provider_os_memory.c b/src/provider/provider_os_memory.c
index 87dd08cf7..8c5a9dc46 100644
--- a/src/provider/provider_os_memory.c
+++ b/src/provider/provider_os_memory.c
@@ -341,6 +341,22 @@ validatePartitions(umf_os_memory_provider_params_t *params) {
     return UMF_RESULT_SUCCESS;
 }
 
+static umf_result_t os_get_min_page_size(void *provider, void *ptr,
+                                         size_t *page_size);
+
+static umf_result_t validatePartSize(os_memory_provider_t *provider,
+                                     umf_os_memory_provider_params_t *params) {
+    size_t page_size;
+    os_get_min_page_size(provider, NULL, &page_size);
+    if (ALIGN_UP(params->part_size, page_size) < params->part_size) {
+        LOG_ERR("partition size (%zu) is too big, cannot align with a page "
+                "size (%zu)",
+                params->part_size, page_size);
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+    return UMF_RESULT_SUCCESS;
+}
+
 static void free_bitmaps(os_memory_provider_t *provider) {
     for (unsigned i = 0; i < provider->nodeset_len; i++) {
         hwloc_bitmap_free(provider->nodeset[i]);
@@ -427,6 +443,14 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
         return result;
     }
 
+    if (in_params->numa_mode == UMF_NUMA_MODE_INTERLEAVE) {
+        result = validatePartSize(provider, in_params);
+        if (result != UMF_RESULT_SUCCESS) {
+            LOG_ERR("incorrect partition size: %zu", in_params->part_size);
+            return result;
+        }
+    }
+
     int is_dedicated_node_bind = dedicated_node_bind(in_params);
     provider->numa_policy =
         translate_numa_mode(in_params->numa_mode, is_dedicated_node_bind);
@@ -574,9 +598,6 @@ static void os_finalize(void *provider) {
     umf_ba_global_free(os_provider);
 }
 
-static umf_result_t os_get_min_page_size(void *provider, void *ptr,
-                                         size_t *page_size);
-
 // TODO: this function should be re-enabled when CTL is implemented
 #if 0
 static void print_numa_nodes(os_memory_provider_t *os_provider, void *addr,
@@ -813,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;
     }
@@ -924,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;
         }
diff --git a/src/proxy_lib/proxy_lib.c b/src/proxy_lib/proxy_lib.c
index ca8d69315..c654655ba 100644
--- a/src/proxy_lib/proxy_lib.c
+++ b/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) {
diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h
index 25a840d97..eebc461f6 100644
--- a/src/utils/utils_common.h
+++ b/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 patch:
diff --git a/test/common/provider.hpp b/test/common/provider.hpp
index 11c7056db..148f34dc8 100644
--- a/test/common/provider.hpp
+++ b/test/common/provider.hpp
@@ -109,7 +109,10 @@ struct provider_ba_global : public provider_base_t {
         // 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);
 
diff --git a/test/common/test_helpers.h b/test/common/test_helpers.h
index 4a581bc4d..494528b57 100644
--- a/test/common/test_helpers.h
+++ b/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);
diff --git a/test/poolFixtures.hpp b/test/poolFixtures.hpp
index 93d839391..e1c1cc722 100644
--- a/test/poolFixtures.hpp
+++ b/test/poolFixtures.hpp
@@ -451,4 +451,9 @@ TEST_P(umfPoolTest, realloc_compliance) {
 
 TEST_P(umfPoolTest, free_compliance) { free_compliance_test(pool.get()); }
 
+TEST_P(umfPoolTest, allocMaxSize) {
+    auto *ptr = umfPoolMalloc(pool.get(), SIZE_MAX);
+    ASSERT_EQ(ptr, nullptr);
+}
+
 #endif /* UMF_TEST_POOL_FIXTURES_HPP */
diff --git a/test/provider_os_memory.cpp b/test/provider_os_memory.cpp
index 734ebeec9..d0f24d617 100644
--- a/test/provider_os_memory.cpp
+++ b/test/provider_os_memory.cpp
@@ -260,6 +260,12 @@ TEST_P(umfProviderTest, alloc_WRONG_SIZE) {
                        UMF_OS_RESULT_ERROR_ALLOC_FAILED);
 }
 
+TEST_P(umfProviderTest, alloc_MAX_SIZE) {
+    test_alloc_failure(provider.get(), SIZE_MAX, 0,
+                       UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC,
+                       UMF_OS_RESULT_ERROR_ALLOC_FAILED);
+}
+
 // other positive tests
 
 TEST_P(umfProviderTest, get_min_page_size) {
diff --git a/test/provider_os_memory_multiple_numa_nodes.cpp b/test/provider_os_memory_multiple_numa_nodes.cpp
index 8c771a642..7f0a1401b 100644
--- a/test/provider_os_memory_multiple_numa_nodes.cpp
+++ b/test/provider_os_memory_multiple_numa_nodes.cpp
@@ -774,3 +774,21 @@ TEST_F(testNuma, checkModeInterleaveIllegalArgSet) {
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
     ASSERT_EQ(os_memory_provider, nullptr);
 }
+
+// Interleave mode set with SIZE_MAX part size
+TEST_F(testNuma, maxPartSize) {
+    std::vector<unsigned> numa_nodes = get_available_numa_nodes();
+
+    umf_os_memory_provider_params_t os_memory_provider_params =
+        UMF_OS_MEMORY_PROVIDER_PARAMS_TEST;
+    os_memory_provider_params.numa_mode = UMF_NUMA_MODE_INTERLEAVE;
+    os_memory_provider_params.part_size = SIZE_MAX;
+    os_memory_provider_params.numa_list = numa_nodes.data();
+    os_memory_provider_params.numa_list_len = numa_nodes.size();
+
+    auto res = umfMemoryProviderCreate(umfOsMemoryProviderOps(),
+                                       &os_memory_provider_params,
+                                       &os_memory_provider);
+    ASSERT_EQ(res, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+    ASSERT_EQ(os_memory_provider, nullptr);
+}
diff --git a/test/supp/helgrind-umf_test-disjointPool.supp b/test/supp/helgrind-umf_test-disjointPool.supp
index 917237d7e..3ada32736 100644
--- a/test/supp/helgrind-umf_test-disjointPool.supp
+++ b/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*
+   ...
+}
diff --git a/test/test_valgrind.sh b/test/test_valgrind.sh
index ae922f870..9f84cf0d3 100755
--- a/test/test_valgrind.sh
+++ b/test/test_valgrind.sh
@@ -125,6 +125,9 @@ for test in $(ls -1 umf_test-*); do
 	umf_test-memspace_lowest_latency)
 		FILTER='--gtest_filter="-*allocLocalMt*"'
 		;;
+	umf_test-memoryPool)
+		FILTER='--gtest_filter="-*allocMaxSize*"'
+		;;
 	esac
 
 	[ "$FILTER" != "" ] && echo -n "($FILTER) "