Skip to content

Commit 4cb4024

Browse files
PatKaminPatKamin
committed
Align values up safely
1 parent 2397c91 commit 4cb4024

File tree

8 files changed

+71
-21
lines changed

8 files changed

+71
-21
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: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,12 +851,20 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
851851
membind_t membind;
852852
memset(&membind, 0, sizeof(membind));
853853

854-
membind.alloc_size = ALIGN_UP(size, page_size);
854+
membind.alloc_size = ALIGN_UP_SAFE(size, page_size);
855+
if (membind.alloc_size == 0) {
856+
LOG_ERR("size is too big, page align failed");
857+
return membind;
858+
}
855859
membind.page_size = page_size;
856860
membind.addr = addr;
857861
membind.pages = membind.alloc_size / membind.page_size;
858862
if (provider->nodeset_len == 1) {
859-
membind.bind_size = ALIGN_UP(size, membind.page_size);
863+
membind.bind_size = ALIGN_UP_SAFE(size, membind.page_size);
864+
if (membind.bind_size == 0) {
865+
LOG_ERR("size is too big, page align failed");
866+
return membind;
867+
}
860868
membind.bitmap = provider->nodeset[0];
861869
return membind;
862870
}
@@ -866,7 +874,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
866874
size_t s = util_fetch_and_add64(&provider->alloc_sum, size);
867875
membind.node = (s / provider->part_size) % provider->nodeset_len;
868876
membind.bitmap = provider->nodeset[membind.node];
869-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
877+
membind.bind_size =
878+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
879+
if (membind.bind_size == 0) {
880+
LOG_ERR("size is too big, page align failed");
881+
return membind;
882+
}
870883
if (membind.bind_size > membind.alloc_size) {
871884
membind.bind_size = membind.alloc_size;
872885
}
@@ -902,7 +915,12 @@ static membind_t membindNext(os_memory_provider_t *provider,
902915
membind.node++;
903916
membind.node %= provider->nodeset_len;
904917
membind.bitmap = provider->nodeset[membind.node];
905-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
918+
membind.bind_size =
919+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
920+
if (membind.bind_size == 0) {
921+
LOG_ERR("part_size is too big, page align failed");
922+
return membind;
923+
}
906924
if (membind.bind_size > membind.alloc_size) {
907925
membind.bind_size = membind.alloc_size;
908926
}

‎src/proxy_lib/proxy_lib.c‎

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

238238
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
@@ -29,8 +29,13 @@ extern "C" {
2929
expression; \
3030
} while (0)
3131

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

3540
#define VALGRIND_ANNOTATE_NEW_MEMORY(p, s) DO_WHILE_EMPTY
3641
#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
@@ -104,15 +104,14 @@ struct provider_malloc : public provider_base_t {
104104
align = 8;
105105
}
106106

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

117116
#ifdef _WIN32
118117
*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
@@ -75,8 +75,12 @@ static inline void UT_OUT(const char *format, ...) {
7575
(unsigned long long)(rhs)), \
7676
0)))
7777

78-
#ifndef ALIGN_UP
79-
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
78+
#ifndef ALIGN_UP_SAFE
79+
#define ALIGN_UP_SAFE(value, alignment) \
80+
((alignment == 0) ? (value) \
81+
: ((value + alignment - 1) < value \
82+
? 0 \
83+
: ((value + alignment - 1) & ~(alignment - 1))))
8084
#endif
8185

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

0 commit comments

Comments
 (0)