diff --git a/.github/workflows/reusable_sanitizers.yml b/.github/workflows/reusable_sanitizers.yml index 2c63ebd51b..3acda6833e 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 7a98684c64..209ace7fe3 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 ec6bc9fcbf..11d88b731a 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 8773e5cab2..a35a6c2431 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 87dd08cf73..8c5a9dc46a 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 ca8d693158..c654655ba4 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 25a840d978..eebc461f62 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))) diff --git a/test/common/provider.hpp b/test/common/provider.hpp index 11c7056db2..148f34dc89 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 4a581bc4d3..494528b57e 100644 --- a/test/common/test_helpers.h +++ b/test/common/test_helpers.h @@ -14,6 +14,7 @@ #include #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 93d8393916..e1c1cc7225 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 734ebeec9b..d0f24d617c 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 8c771a642c..7f0a1401bc 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 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 917237d7e9..3ada32736c 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 ae922f8701..9f84cf0d32 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) "