Skip to content

Commit dfc01f3

Browse files
PatKaminPatKamin
committed
Align values up safely
Co-authored-by: [email protected]
1 parent 52f2879 commit dfc01f3

File tree

8 files changed

+70
-19
lines changed

8 files changed

+70
-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 == 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: 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: pointer 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: 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: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -834,12 +834,20 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
834834
membind_t membind;
835835
memset(&membind, 0, sizeof(membind));
836836

837-
membind.alloc_size = ALIGN_UP(size, page_size);
837+
membind.alloc_size = ALIGN_UP_SAFE(size, page_size);
838+
if (membind.alloc_size == 0) {
839+
LOG_ERR("size is too big, page align failed");
840+
return membind;
841+
}
838842
membind.page_size = page_size;
839843
membind.addr = addr;
840844
membind.pages = membind.alloc_size / membind.page_size;
841845
if (provider->nodeset_len == 1) {
842-
membind.bind_size = ALIGN_UP(size, membind.page_size);
846+
membind.bind_size = ALIGN_UP_SAFE(size, membind.page_size);
847+
if (membind.bind_size == 0) {
848+
LOG_ERR("size is too big, page align failed");
849+
return membind;
850+
}
843851
membind.bitmap = provider->nodeset[0];
844852
return membind;
845853
}
@@ -849,7 +857,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
849857
size_t s = utils_fetch_and_add64(&provider->alloc_sum, size);
850858
membind.node = (s / provider->part_size) % provider->nodeset_len;
851859
membind.bitmap = provider->nodeset[membind.node];
852-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
860+
membind.bind_size =
861+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
862+
if (membind.bind_size == 0) {
863+
LOG_ERR("size is too big, page align failed");
864+
return membind;
865+
}
853866
if (membind.bind_size > membind.alloc_size) {
854867
membind.bind_size = membind.alloc_size;
855868
}
@@ -885,7 +898,12 @@ static membind_t membindNext(os_memory_provider_t *provider,
885898
membind.node++;
886899
membind.node %= provider->nodeset_len;
887900
membind.bitmap = provider->nodeset[membind.node];
888-
membind.bind_size = ALIGN_UP(provider->part_size, membind.page_size);
901+
membind.bind_size =
902+
ALIGN_UP_SAFE(provider->part_size, membind.page_size);
903+
if (membind.bind_size == 0) {
904+
LOG_ERR("part_size is too big, page align failed");
905+
return membind;
906+
}
889907
if (membind.bind_size > membind.alloc_size) {
890908
membind.bind_size = membind.alloc_size;
891909
}

‎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 > SIZE_MAX - align + 1) {
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 == 0) {
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, align) \
80+
(((align) == 0) ? (value) \
81+
: (((value) + (align)-1) < (value) \
82+
? 0 \
83+
: (((value) + (align)-1) & ~((align)-1))))
8084
#endif
8185

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

0 commit comments

Comments
 (0)