Skip to content

Commit ac823bb

Browse files
lukaszstolarczuklukaszstolarczuk
authored
Merge pull request #587 from PatKamin/integer-overflow-tests
Add overflow tests
2 parents b7de6c5 + 4859b62 commit ac823bb

14 files changed

+146
-19
lines changed

‎.github/workflows/reusable_sanitizers.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ jobs:
7373
7474
- name: Run tests
7575
working-directory: ${{env.BUILD_DIR}}
76+
env:
77+
ASAN_OPTIONS: allocator_may_return_null=1
78+
TSAN_OPTIONS: allocator_may_return_null=1
7679
run: |
7780
${{ matrix.compiler.cxx == 'icpx' && '. /opt/intel/oneapi/setvars.sh' || true }}
7881
ctest --output-on-failure
@@ -141,4 +144,7 @@ jobs:
141144

142145
- name: Run tests
143146
working-directory: ${{env.BUILD_DIR}}
147+
env:
148+
ASAN_OPTIONS: allocator_may_return_null=1
149+
TSAN_OPTIONS: allocator_may_return_null=1
144150
run: ctest -C Debug --output-on-failure

‎src/base_alloc/base_alloc.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ static void *ba_os_alloc_annotated(size_t pool_size) {
134134
}
135135

136136
umf_ba_pool_t *umf_ba_create(size_t size) {
137-
size_t chunk_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
137+
size_t chunk_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
138+
if (chunk_size == 0) {
139+
return NULL;
140+
}
138141
size_t mutex_size = ALIGN_UP(utils_mutex_get_size(), MEMORY_ALIGNMENT);
139142

140143
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) {
144147
pool_size = MINIMUM_POOL_SIZE;
145148
}
146149

147-
pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
150+
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
151+
if (pool_size == 0) {
152+
return NULL;
153+
}
148154

149155
umf_ba_pool_t *pool = (umf_ba_pool_t *)ba_os_alloc_annotated(pool_size);
150156
if (!pool) {

‎src/base_alloc/base_alloc_global.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,12 @@ static void *add_metadata_and_align(void *ptr, size_t size, size_t alignment) {
9696
if (alignment <= ALLOC_METADATA_SIZE) {
9797
user_ptr = (void *)((uintptr_t)ptr + ALLOC_METADATA_SIZE);
9898
} else {
99-
user_ptr =
100-
(void *)ALIGN_UP((uintptr_t)ptr + ALLOC_METADATA_SIZE, alignment);
99+
user_ptr = (void *)ALIGN_UP_SAFE((uintptr_t)ptr + ALLOC_METADATA_SIZE,
100+
alignment);
101+
if (!user_ptr) {
102+
LOG_ERR("base_alloc: pointer alignment overflow");
103+
return NULL;
104+
}
101105
}
102106

103107
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) {
155159
return NULL;
156160
}
157161

162+
if (size > SIZE_MAX - ALLOC_METADATA_SIZE) {
163+
LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
164+
return NULL;
165+
}
166+
158167
// for metadata
159168
size += ALLOC_METADATA_SIZE;
160169

161170
if (alignment > ALLOC_METADATA_SIZE) {
171+
if (size > SIZE_MAX - alignment) {
172+
LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
173+
return NULL;
174+
}
162175
size += alignment;
163176
}
164177

‎src/base_alloc/base_alloc_linear.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,11 @@ umf_ba_linear_pool_t *umf_ba_linear_create(size_t pool_size) {
8888
pool_size = MINIMUM_LINEAR_POOL_SIZE;
8989
}
9090

91-
pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
91+
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
92+
if (pool_size == 0) {
93+
LOG_ERR("pool_size page alignment overflow");
94+
return NULL;
95+
}
9296

9397
umf_ba_linear_pool_t *pool = (umf_ba_linear_pool_t *)ba_os_alloc(pool_size);
9498
if (!pool) {
@@ -122,15 +126,24 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) {
122126
if (size == 0) {
123127
return NULL;
124128
}
125-
size_t aligned_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
129+
size_t aligned_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
130+
if (aligned_size == 0) {
131+
LOG_ERR("size alignment overflow");
132+
return NULL;
133+
}
126134
utils_mutex_lock(&pool->metadata.lock);
127135
if (pool->metadata.size_left < aligned_size) {
128136
size_t pool_size = MINIMUM_LINEAR_POOL_SIZE;
129137
size_t usable_size =
130138
pool_size - offsetof(umf_ba_next_linear_pool_t, data);
131139
if (usable_size < aligned_size) {
132140
pool_size += aligned_size - usable_size;
133-
pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
141+
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
142+
if (pool_size == 0) {
143+
utils_mutex_unlock(&pool->metadata.lock);
144+
LOG_ERR("pool_size page alignment overflow");
145+
return NULL;
146+
}
134147
}
135148

136149
assert(pool_size - offsetof(umf_ba_next_linear_pool_t, data) >=

‎src/provider/provider_os_memory.c

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,22 @@ validatePartitions(umf_os_memory_provider_params_t *params) {
341341
return UMF_RESULT_SUCCESS;
342342
}
343343

344+
static umf_result_t os_get_min_page_size(void *provider, void *ptr,
345+
size_t *page_size);
346+
347+
static umf_result_t validatePartSize(os_memory_provider_t *provider,
348+
umf_os_memory_provider_params_t *params) {
349+
size_t page_size;
350+
os_get_min_page_size(provider, NULL, &page_size);
351+
if (ALIGN_UP(params->part_size, page_size) < params->part_size) {
352+
LOG_ERR("partition size (%zu) is too big, cannot align with a page "
353+
"size (%zu)",
354+
params->part_size, page_size);
355+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
356+
}
357+
return UMF_RESULT_SUCCESS;
358+
}
359+
344360
static void free_bitmaps(os_memory_provider_t *provider) {
345361
for (unsigned i = 0; i < provider->nodeset_len; i++) {
346362
hwloc_bitmap_free(provider->nodeset[i]);
@@ -427,6 +443,14 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
427443
return result;
428444
}
429445

446+
if (in_params->numa_mode == UMF_NUMA_MODE_INTERLEAVE) {
447+
result = validatePartSize(provider, in_params);
448+
if (result != UMF_RESULT_SUCCESS) {
449+
LOG_ERR("incorrect partition size: %zu", in_params->part_size);
450+
return result;
451+
}
452+
}
453+
430454
int is_dedicated_node_bind = dedicated_node_bind(in_params);
431455
provider->numa_policy =
432456
translate_numa_mode(in_params->numa_mode, is_dedicated_node_bind);
@@ -574,9 +598,6 @@ static void os_finalize(void *provider) {
574598
umf_ba_global_free(os_provider);
575599
}
576600

577-
static umf_result_t os_get_min_page_size(void *provider, void *ptr,
578-
size_t *page_size);
579-
580601
// TODO: this function should be re-enabled when CTL is implemented
581602
#if 0
582603
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,
813834
membind_t membind;
814835
memset(&membind, 0, sizeof(membind));
815836

816-
membind.alloc_size = ALIGN_UP(size, page_size);
837+
membind.alloc_size = size;
817838
membind.page_size = page_size;
818839
membind.addr = addr;
819840
membind.pages = membind.alloc_size / membind.page_size;
820841
if (provider->nodeset_len == 1) {
821-
membind.bind_size = ALIGN_UP(size, membind.page_size);
842+
membind.bind_size = size;
822843
membind.bitmap = provider->nodeset[0];
823844
return membind;
824845
}
@@ -924,7 +945,15 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
924945

925946
// Bind memory to NUMA nodes if numa_policy is other than DEFAULT
926947
if (os_provider->numa_policy != HWLOC_MEMBIND_DEFAULT) {
927-
membind_t membind = membindFirst(os_provider, addr, size, page_size);
948+
size_t first_size = ALIGN_UP_SAFE(size, page_size);
949+
if (first_size == 0) {
950+
LOG_ERR("size is too big, page align failed");
951+
(void)utils_munmap(addr, size);
952+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
953+
}
954+
955+
membind_t membind =
956+
membindFirst(os_provider, addr, first_size, page_size);
928957
if (membind.bitmap == NULL) {
929958
goto err_unmap;
930959
}

‎src/proxy_lib/proxy_lib.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static inline void *ba_leak_realloc(void *ptr, size_t size, size_t max_size) {
228228
static inline void *ba_leak_aligned_alloc(size_t alignment, size_t size) {
229229
ba_leak_init_once();
230230
void *ptr = umf_ba_linear_alloc(Base_alloc_leak, size + alignment);
231-
return (void *)ALIGN_UP((uintptr_t)ptr, alignment);
231+
return (void *)ALIGN_UP_SAFE((uintptr_t)ptr, alignment);
232232
}
233233

234234
static inline int ba_leak_free(void *ptr) {

‎src/utils/utils_common.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ typedef enum umf_purge_advise_t {
4040
#define IS_NOT_ALIGNED(value, align) \
4141
((align != 0 && (((value) & ((align)-1)) != 0)))
4242
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
43+
#define ALIGN_UP_SAFE(value, align) \
44+
(((align) == 0) \
45+
? (value) \
46+
: (((value) + (align)-1) < (value) ? 0 : ALIGN_UP((value), (align))))
4347
#define ALIGN_DOWN(value, align) ((value) & ~((align)-1))
4448
#define ASSERT_IS_ALIGNED(value, align) \
4549
DO_WHILE_EXPRS(assert(IS_ALIGNED(value, align)))

‎test/common/provider.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ struct provider_ba_global : public provider_base_t {
109109
// requirement of 'size' being multiple of 'align' even though the
110110
// documentation says that it has to. AddressSanitizer returns an
111111
// error because of this issue.
112-
size_t aligned_size = ALIGN_UP(size, align);
112+
size_t aligned_size = ALIGN_UP_SAFE(size, align);
113+
if (aligned_size == 0) {
114+
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
115+
}
113116

114117
*ptr = umf_ba_global_aligned_alloc(aligned_size, align);
115118

‎test/common/test_helpers.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <umf/memory_provider_ops.h>
1515

1616
#include "provider_trace.h"
17+
#include "utils_common.h"
1718

1819
#ifdef __cplusplus
1920
extern "C" {
@@ -22,10 +23,6 @@ extern "C" {
2223
// Needed for CI
2324
#define TEST_SKIP_ERROR_CODE 125
2425

25-
#ifndef ALIGN_UP
26-
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
27-
#endif
28-
2926
int bufferIsFilledWithChar(void *ptr, size_t size, char c);
3027

3128
int buffersHaveSameContent(void *first, void *second, size_t size);

‎test/poolFixtures.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,4 +451,9 @@ TEST_P(umfPoolTest, realloc_compliance) {
451451

452452
TEST_P(umfPoolTest, free_compliance) { free_compliance_test(pool.get()); }
453453

454+
TEST_P(umfPoolTest, allocMaxSize) {
455+
auto *ptr = umfPoolMalloc(pool.get(), SIZE_MAX);
456+
ASSERT_EQ(ptr, nullptr);
457+
}
458+
454459
#endif /* UMF_TEST_POOL_FIXTURES_HPP */

0 commit comments

Comments
 (0)