Skip to content

Commit 23b6809

Browse files
PatKaminPatKamin
committed
Align values up safely
1 parent 66c2df7 commit 23b6809

File tree

8 files changed

+72
-22
lines changed

8 files changed

+72
-22
lines changed

‎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) {
139+
return NULL;
140+
}
138141
size_t mutex_size = ALIGN_UP(util_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) {
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: 6 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: ptr alignment overflow");
103+
return NULL;
104+
}
101105
}
102106

103107
size_t ptr_offset_from_original = (uintptr_t)user_ptr - (uintptr_t)ptr;

‎src/base_alloc/base_alloc_linear.c‎

Lines changed: 17 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) {
93+
LOG_ERR("umf_ba_linear_create: 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,25 @@ 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) {
131+
LOG_ERR("umf_ba_linear_alloc: size alignment overflow");
132+
return NULL;
133+
}
126134
util_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) {
143+
util_mutex_unlock(&pool->metadata.lock);
144+
LOG_ERR(
145+
"umf_ba_linear_alloc: pool_size page alignment overflow");
146+
return NULL;
147+
}
134148
}
135149

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

‎src/provider/provider_os_memory.c‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ static umf_result_t validatePartSize(os_memory_provider_t *provider,
367367
umf_os_memory_provider_params_t *params) {
368368
size_t page_size;
369369
os_get_min_page_size(provider, NULL, &page_size);
370-
if (ALIGN_UP(params->part_size, page_size) < params->part_size) {
370+
if (!ALIGN_UP(params->part_size, page_size)) {
371371
LOG_ERR("partition size (%zu) is too big, cannot align with a page "
372372
"size (%zu)",
373373
params->part_size, page_size);
@@ -829,12 +829,20 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
829829
membind_t membind;
830830
memset(&membind, 0, sizeof(membind));
831831

832-
membind.alloc_size = ALIGN_UP(size, page_size);
832+
membind.alloc_size = ALIGN_UP_SAFE(size, page_size);
833+
if (membind.alloc_size == 0) {
834+
LOG_ERR("size is too big, page align failed");
835+
return membind;
836+
}
833837
membind.page_size = page_size;
834838
membind.addr = addr;
835839
membind.pages = membind.alloc_size / membind.page_size;
836840
if (provider->nodeset_len == 1) {
837-
membind.bind_size = ALIGN_UP(size, membind.page_size);
841+
membind.bind_size = ALIGN_UP_SAFE(size, membind.page_size);
842+
if (membind.bind_size == 0) {
843+
LOG_ERR("size is too big, page align failed");
844+
return membind;
845+
}
838846
membind.bitmap = provider->nodeset[0];
839847
return membind;
840848
}
@@ -844,7 +852,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
844852
size_t s = util_fetch_and_add64(&provider->alloc_sum, size);
845853
membind.node = (s / provider->part_size) % provider->nodeset_len;
846854
membind.bitmap = provider->nodeset[membind.node];
847-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
855+
membind.bind_size =
856+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
857+
if (membind.bind_size == 0) {
858+
LOG_ERR("size is too big, page align failed");
859+
return membind;
860+
}
848861
if (membind.bind_size > membind.alloc_size) {
849862
membind.bind_size = membind.alloc_size;
850863
}
@@ -880,7 +893,12 @@ static membind_t membindNext(os_memory_provider_t *provider,
880893
membind.node++;
881894
membind.node %= provider->nodeset_len;
882895
membind.bitmap = provider->nodeset[membind.node];
883-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
896+
membind.bind_size =
897+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
898+
if (membind.bind_size == 0) {
899+
LOG_ERR("part_size is too big, page align failed");
900+
return membind;
901+
}
884902
if (membind.bind_size > membind.alloc_size) {
885903
membind.bind_size = membind.alloc_size;
886904
}

‎src/proxy_lib/proxy_lib.c‎

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

237237
static inline int ba_leak_free(void *ptr) {

‎src/utils/utils_common.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,13 @@ extern "C" {
2727
expression; \
2828
} while (0)
2929

30-
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
31-
#define ALIGN_DOWN(value, align) ((value) & ~((align)-1))
30+
#define ALIGN_UP(value, align) ((value + align - 1) & ~(align - 1))
31+
#define ALIGN_UP_SAFE(value, alignment) \
32+
((alignment == 0) ? (value) \
33+
: ((value + alignment - 1) < value \
34+
? 0 \
35+
: ((value + alignment - 1) & ~(alignment - 1))))
36+
#define ALIGN_DOWN(value, align) (value & ~(align - 1))
3237

3338
#define VALGRIND_ANNOTATE_NEW_MEMORY(p, s) DO_WHILE_EMPTY
3439
#define VALGRIND_HG_DRD_DISABLE_CHECKING(p, s) DO_WHILE_EMPTY

‎test/common/provider.hpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,14 @@ struct provider_malloc : public provider_base_t {
106106
align = 8;
107107
}
108108

109-
if (SIZE_MAX - size < align) {
110-
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
111-
}
112-
113109
// aligned_malloc returns a valid pointer despite not meeting the
114110
// requirement of 'size' being multiple of 'align' even though the
115111
// documentation says that it has to. AddressSanitizer returns an
116112
// error because of this issue.
117-
size_t aligned_size = ALIGN_UP(size, align);
113+
size_t aligned_size = ALIGN_UP_SAFE(size, align);
114+
if (!aligned_size) {
115+
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
116+
}
118117

119118
#ifdef _WIN32
120119
*ptr = _aligned_malloc(aligned_size, align);

‎test/common/test_helpers.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,12 @@ static inline void UT_OUT(const char *format, ...) {
7272
(unsigned long long)(rhs)), \
7373
0)))
7474

75-
#ifndef ALIGN_UP
76-
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
75+
#ifndef ALIGN_UP_SAFE
76+
#define ALIGN_UP_SAFE(value, alignment) \
77+
((alignment == 0) ? (value) \
78+
: ((value + alignment - 1) < value \
79+
? 0 \
80+
: ((value + alignment - 1) & ~(alignment - 1))))
7781
#endif
7882

7983
int bufferIsFilledWithChar(void *ptr, size_t size, char c);

0 commit comments

Comments
 (0)