From 4a97c2c4b985f96072b162da8343e9d5aadaf1bb Mon Sep 17 00:00:00 2001 From: Stanislav Paskalev Date: Mon, 23 Sep 2024 00:15:24 +0300 Subject: [PATCH] #119 return a status from buddy_safe_free (#120) --- buddy_alloc.h | 88 +++++++++++++++++++++++++++++++++++---------------- tests.c | 47 +++++++++++++++------------ 2 files changed, 87 insertions(+), 48 deletions(-) diff --git a/buddy_alloc.h b/buddy_alloc.h index d0ef7dd..483723d 100644 --- a/buddy_alloc.h +++ b/buddy_alloc.h @@ -114,8 +114,16 @@ void *buddy_reallocarray(struct buddy *buddy, void *ptr, /* Use the specified buddy to free memory. See free. */ void buddy_free(struct buddy *buddy, void *ptr); +enum buddy_safe_free_status { + BUDDY_SAFE_FREE_SUCCESS, + BUDDY_SAFE_FREE_BUDDY_IS_NULL, + BUDDY_SAFE_FREE_INVALID_ADDRESS, + BUDDY_SAFE_FREE_SIZE_MISMATCH, + BUDDY_SAFE_FREE_ALREADY_FREE, +}; + /* A (safer) free with a size. Will not free unless the size fits the target span. */ -void buddy_safe_free(struct buddy *buddy, void *ptr, size_t requested_size); +enum buddy_safe_free_status buddy_safe_free(struct buddy *buddy, void *ptr, size_t requested_size); /* * Reservation functions @@ -321,8 +329,13 @@ static size_t buddy_tree_status(struct buddy_tree *t, struct buddy_tree_pos pos) /* Marks the indicated position as allocated and propagates the change */ static void buddy_tree_mark(struct buddy_tree *t, struct buddy_tree_pos pos); +enum buddy_tree_release_status { + BUDDY_TREE_RELEASE_SUCCESS, + BUDDY_TREE_RELEASE_FAIL_PARTIALLY_USED, +}; + /* Marks the indicated position as free and propagates the change */ -static void buddy_tree_release(struct buddy_tree *t, struct buddy_tree_pos pos); +static enum buddy_tree_release_status buddy_tree_release(struct buddy_tree *t, struct buddy_tree_pos pos); /* Returns a free position at the specified depth or an invalid position */ static struct buddy_tree_pos buddy_tree_find_free(struct buddy_tree *t, uint8_t depth); @@ -849,30 +862,31 @@ void buddy_free(struct buddy *buddy, void *ptr) { buddy_tree_release(tree, pos); } -void buddy_safe_free(struct buddy *buddy, void *ptr, size_t requested_size) { - unsigned char *dst, *main; - struct buddy_tree *tree; +enum buddy_safe_free_status buddy_safe_free(struct buddy* buddy, void* ptr, size_t requested_size) { + unsigned char* dst, * main; + struct buddy_tree* tree; struct buddy_tree_pos pos; size_t allocated_size_for_depth; + enum buddy_tree_release_status status; if (buddy == NULL) { - return; + return BUDDY_SAFE_FREE_BUDDY_IS_NULL; } if (ptr == NULL) { - return; + return BUDDY_SAFE_FREE_INVALID_ADDRESS; } - dst = (unsigned char *)ptr; + dst = (unsigned char*)ptr; main = buddy_main(buddy); if ((dst < main) || (dst >= (main + buddy->memory_size))) { - return; + return BUDDY_SAFE_FREE_INVALID_ADDRESS; } - /* Find the position tracking this address */ + /* Find an allocated position tracking this address */ tree = buddy_tree(buddy); pos = position_for_address(buddy, dst); - if (! buddy_tree_valid(tree, pos)) { - return; + if (!buddy_tree_valid(tree, pos)) { + return BUDDY_SAFE_FREE_INVALID_ADDRESS; } allocated_size_for_depth = size_for_depth(buddy, pos.depth); @@ -880,14 +894,23 @@ void buddy_safe_free(struct buddy *buddy, void *ptr, size_t requested_size) { requested_size = buddy->alignment; } if (requested_size > allocated_size_for_depth) { - return; + return BUDDY_SAFE_FREE_SIZE_MISMATCH; } if (requested_size <= (allocated_size_for_depth / 2)) { - return; + return BUDDY_SAFE_FREE_SIZE_MISMATCH; } /* Release the position */ - buddy_tree_release(tree, pos); + status = buddy_tree_release(tree, pos); + + switch (status) { + case BUDDY_TREE_RELEASE_FAIL_PARTIALLY_USED: + return BUDDY_SAFE_FREE_INVALID_ADDRESS; + case BUDDY_TREE_RELEASE_SUCCESS: + break; + } + + return BUDDY_SAFE_FREE_SUCCESS; } void buddy_reserve_range(struct buddy *buddy, void *ptr, size_t requested_size) { @@ -1062,7 +1085,6 @@ static unsigned int buddy_relative_mode(struct buddy *buddy) { static void buddy_toggle_virtual_slots(struct buddy *buddy, unsigned int state) { size_t delta, memory_size, effective_memory_size; - void (*toggle)(struct buddy_tree *, struct buddy_tree_pos); struct buddy_tree *tree; struct buddy_tree_pos pos; @@ -1077,16 +1099,18 @@ static void buddy_toggle_virtual_slots(struct buddy *buddy, unsigned int state) /* Node memory size is already aligned to buddy->alignment */ delta = effective_memory_size - memory_size; - /* Determine whether to mark or release */ - toggle = state ? &buddy_tree_mark : &buddy_tree_release; - tree = buddy_tree(buddy); pos = buddy_tree_right_child(buddy_tree_root()); while (delta) { size_t current_pos_size = size_for_depth(buddy, buddy_tree_depth(pos)); if (delta == current_pos_size) { /* toggle current pos */ - (*toggle)(tree, pos); + if (state) { + buddy_tree_mark(tree, pos); + } + else { + buddy_tree_release(tree, pos); + } break; } if (delta <= (current_pos_size / 2)) { @@ -1095,7 +1119,12 @@ static void buddy_toggle_virtual_slots(struct buddy *buddy, unsigned int state) continue; } else { /* toggle right child */ - (*toggle)(tree, buddy_tree_right_child(pos)); + if (state) { + buddy_tree_mark(tree, buddy_tree_right_child(pos)); + } + else { + buddy_tree_release(tree, buddy_tree_right_child(pos)); + } /* reduce delta */ delta -= current_pos_size / 2; /* re-run for left child */ @@ -1107,7 +1136,6 @@ static void buddy_toggle_virtual_slots(struct buddy *buddy, unsigned int state) static void buddy_toggle_range_reservation(struct buddy *buddy, void *ptr, size_t requested_size, unsigned int state) { unsigned char *dst, *main; - void (*toggle)(struct buddy_tree *, struct buddy_tree_pos); struct buddy_tree *tree; size_t offset; struct buddy_tree_pos pos; @@ -1127,9 +1155,6 @@ static void buddy_toggle_range_reservation(struct buddy *buddy, void *ptr, size_ return; } - /* Determine whether to mark or release */ - toggle = state ? &buddy_tree_mark : &buddy_tree_release; - /* Find the deepest position tracking this address */ tree = buddy_tree(buddy); offset = (size_t) (dst - main); @@ -1137,7 +1162,12 @@ static void buddy_toggle_range_reservation(struct buddy *buddy, void *ptr, size_ /* Advance one position at a time and process */ while (requested_size) { - (*toggle)(tree, pos); + if (state) { + buddy_tree_mark(tree, pos); + } + else { + buddy_tree_release(tree, pos); + } requested_size = (requested_size < buddy->alignment) ? 0 : (requested_size - buddy->alignment); pos.index++; } @@ -1597,12 +1627,12 @@ static void buddy_tree_mark(struct buddy_tree *t, struct buddy_tree_pos pos) { update_parent_chain(t, pos, internal, internal.local_offset); } -static void buddy_tree_release(struct buddy_tree *t, struct buddy_tree_pos pos) { +static enum buddy_tree_release_status buddy_tree_release(struct buddy_tree *t, struct buddy_tree_pos pos) { /* Calling release on an unused or a partially-used position a bug in caller */ struct internal_position internal = buddy_tree_internal_position_tree(t, pos); if (read_from_internal_position(buddy_tree_bits(t), internal) != internal.local_offset) { - return; + return BUDDY_TREE_RELEASE_FAIL_PARTIALLY_USED; } /* Mark the node as unused */ @@ -1610,6 +1640,8 @@ static void buddy_tree_release(struct buddy_tree *t, struct buddy_tree_pos pos) /* Update the tree upwards */ update_parent_chain(t, pos, internal, 0); + + return BUDDY_TREE_RELEASE_SUCCESS; } static void update_parent_chain(struct buddy_tree *t, struct buddy_tree_pos pos, diff --git a/tests.c b/tests.c index 9cc3d8d..83a373b 100644 --- a/tests.c +++ b/tests.c @@ -345,6 +345,7 @@ void test_buddy_resize_up_within_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 896) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -356,6 +357,7 @@ void test_buddy_resize_up_at_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 1024) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -367,6 +369,7 @@ void test_buddy_resize_up_after_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 2048) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -378,6 +381,7 @@ void test_buddy_resize_down_to_virtual(void) { buddy = buddy_init(buddy_buf, data_buf, 1024); assert(buddy != NULL); assert(buddy_resize(buddy, 832) == buddy); + assert(buddy_resize(buddy, 1024) == buddy); free(buddy_buf); } @@ -389,6 +393,7 @@ void test_buddy_resize_down_to_virtual_partial(void) { buddy = buddy_init(buddy_buf, data_buf, 1024); assert(buddy != NULL); assert(buddy_resize(buddy, 832-1) == buddy); + assert(buddy_resize(buddy, 1024) == buddy); free(buddy_buf); } @@ -400,6 +405,7 @@ void test_buddy_resize_down_within_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 640) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -428,6 +434,7 @@ void test_buddy_resize_down_at_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 512) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -439,6 +446,7 @@ void test_buddy_resize_down_before_reserved(void) { buddy = buddy_init(buddy_buf, data_buf, 768); assert(buddy != NULL); assert(buddy_resize(buddy, 448) == buddy); + assert(buddy_resize(buddy, 768) == buddy); free(buddy_buf); } @@ -883,11 +891,11 @@ void test_buddy_safe_free_coverage(void) { start_test; buddy = buddy_init(buddy_buf, data_buf+1024, 1024); assert(buddy != NULL); - buddy_safe_free(NULL, NULL, 0); - buddy_safe_free(NULL, data_buf+1024, 0); - buddy_safe_free(buddy, NULL, 0); - buddy_safe_free(buddy, data_buf, 0); - buddy_safe_free(buddy, data_buf+2048, 0); + assert(buddy_safe_free(NULL, NULL, 0) == BUDDY_SAFE_FREE_BUDDY_IS_NULL); + assert(buddy_safe_free(buddy, NULL, 0) == BUDDY_SAFE_FREE_INVALID_ADDRESS); + assert(buddy_safe_free(buddy, data_buf, 0) == BUDDY_SAFE_FREE_INVALID_ADDRESS); + assert(buddy_safe_free(buddy, data_buf+2048, 0) == BUDDY_SAFE_FREE_INVALID_ADDRESS); + assert(buddy_safe_free(buddy, data_buf+1024, 1024) == BUDDY_SAFE_FREE_INVALID_ADDRESS); free(buddy_buf); } @@ -897,7 +905,7 @@ void test_buddy_safe_free_alignment(void) { struct buddy *buddy; start_test; buddy = buddy_init(buddy_buf, data_buf, 4096); - buddy_safe_free(buddy, data_buf+1, 0); + assert(buddy_safe_free(buddy, data_buf+1, 0) == BUDDY_SAFE_FREE_INVALID_ADDRESS); free(buddy_buf); } @@ -915,11 +923,11 @@ void test_buddy_safe_free_invalid_free_01(void) { r = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN); assert(r != NULL); assert(l != r); - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // double-free on a right node // that will propagate to a partially-allocated parent - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_INVALID_ADDRESS); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -939,12 +947,12 @@ void test_buddy_safe_free_invalid_free_02(void) { r = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN); assert(r != NULL); assert(l != r); - buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN); - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // double-free on a right node // that will propagate to a unallocated parent - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_INVALID_ADDRESS); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -964,14 +972,14 @@ void test_buddy_safe_free_invalid_free_03(void) { r = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN); assert(r != NULL); assert(l != r); - buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN); - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); m = buddy_malloc(buddy, size); // full allocation assert(m == l); // at the start of the arena memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // double-free on a right node // that will propagate to a fully-allocated parent - buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, r, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_INVALID_ADDRESS); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -991,12 +999,11 @@ void test_buddy_safe_free_invalid_free_04(void) { r = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN); assert(r != NULL); assert(l != r); - buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SUCCESS); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // double-free on a left node // that will propagate to a partially-allocated parent - buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN); - /* the use check in buddy_tree_release handles this case */ + assert(buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN*2) == BUDDY_SAFE_FREE_INVALID_ADDRESS); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -1014,7 +1021,7 @@ void test_buddy_safe_free_invalid_free_05(void) { l = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // safe free with double size - buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN * 2); + assert(buddy_safe_free(buddy, l, BUDDY_ALLOC_ALIGN * 2) == BUDDY_SAFE_FREE_SIZE_MISMATCH); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -1032,7 +1039,7 @@ void test_buddy_safe_free_invalid_free_06(void) { m = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN*2); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // safe free with half size - buddy_safe_free(buddy, m, BUDDY_ALLOC_ALIGN); + assert(buddy_safe_free(buddy, m, BUDDY_ALLOC_ALIGN) == BUDDY_SAFE_FREE_SIZE_MISMATCH); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control); @@ -1050,7 +1057,7 @@ void test_buddy_safe_free_invalid_free_07(void) { m = buddy_malloc(buddy, BUDDY_ALLOC_ALIGN*2); memcpy(buddy_buf_control, buddy_buf, buddy_sizeof(size)); // safe free with zero size - buddy_safe_free(buddy, m, 0); + assert(buddy_safe_free(buddy, m, 0) == BUDDY_SAFE_FREE_SIZE_MISMATCH); assert(memcmp(buddy_buf_control, buddy_buf, buddy_sizeof(size)) == 0); free(buddy_buf); free(buddy_buf_control);