Skip to content

Commit 9ea1868

Browse files
PatKaminPatKamin
committed
Align values up safely
1 parent d9558ed commit 9ea1868

File tree

8 files changed

+71
-19
lines changed

8 files changed

+71
-19
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(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) {
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
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) {
143+
utils_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
@@ -837,12 +837,20 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
837837
membind_t membind;
838838
memset(&membind, 0, sizeof(membind));
839839

840-
membind.alloc_size = ALIGN_UP(size, page_size);
840+
membind.alloc_size = ALIGN_UP_SAFE(size, page_size);
841+
if (membind.alloc_size == 0) {
842+
LOG_ERR("size is too big, page align failed");
843+
return membind;
844+
}
841845
membind.page_size = page_size;
842846
membind.addr = addr;
843847
membind.pages = membind.alloc_size / membind.page_size;
844848
if (provider->nodeset_len == 1) {
845-
membind.bind_size = ALIGN_UP(size, membind.page_size);
849+
membind.bind_size = ALIGN_UP_SAFE(size, membind.page_size);
850+
if (membind.bind_size == 0) {
851+
LOG_ERR("size is too big, page align failed");
852+
return membind;
853+
}
846854
membind.bitmap = provider->nodeset[0];
847855
return membind;
848856
}
@@ -852,7 +860,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
852860
size_t s = utils_fetch_and_add64(&provider->alloc_sum, size);
853861
membind.node = (s / provider->part_size) % provider->nodeset_len;
854862
membind.bitmap = provider->nodeset[membind.node];
855-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
863+
membind.bind_size =
864+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
865+
if (membind.bind_size == 0) {
866+
LOG_ERR("size is too big, page align failed");
867+
return membind;
868+
}
856869
if (membind.bind_size > membind.alloc_size) {
857870
membind.bind_size = membind.alloc_size;
858871
}
@@ -888,7 +901,12 @@ static membind_t membindNext(os_memory_provider_t *provider,
888901
membind.node++;
889902
membind.node %= provider->nodeset_len;
890903
membind.bitmap = provider->nodeset[membind.node];
891-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
904+
membind.bind_size =
905+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
906+
if (membind.bind_size == 0) {
907+
LOG_ERR("part_size is too big, page align failed");
908+
return membind;
909+
}
892910
if (membind.bind_size > membind.alloc_size) {
893911
membind.bind_size = membind.alloc_size;
894912
}

‎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 & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ 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+
#ifndef ALIGN_UP_SAFE
44+
#define ALIGN_UP_SAFE(value, align) \
45+
(((align) == 0) ? (value) \
46+
: (((value) + (align)-1) < (value) \
47+
? 0 \
48+
: (((value) + (align)-1) & ~((align)-1))))
49+
#endif
4350
#define ALIGN_DOWN(value, align) ((value) & ~((align)-1))
4451
#define ASSERT_IS_ALIGNED(value, align) \
4552
DO_WHILE_EXPRS(assert(IS_ALIGNED(value, align)))

‎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)