From d37370301c7cb2af8cf36ce25adc24daeb6c2266 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 19 May 2021 09:50:17 -0700 Subject: [PATCH] k_heap: Clamp to a minimum heap size The K_HEAP_DEFINE macro would allow users to specify heaps that are too small, leading to potential corruption events (though at least there were __ASSERTs that would catch this at runtime if enabled). It would be nice to put the logic to compute this value into the heap code, but that isn't available in kernel.h (and we don't want to pull it in as this header is already WAY to thick). So instead we just hand-compute and document the choice. We can address bitrot problems with a test. (Tweaks to heap size asserts and correct size bounds from Nicolas Pitre) Fixes #33009 Signed-off-by: Andy Ross --- include/kernel.h | 14 ++++++++++++-- lib/os/heap.c | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index c622939cfa8..7aecb86e059 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -5002,6 +5002,11 @@ void *k_heap_alloc(struct k_heap *h, size_t bytes, */ void k_heap_free(struct k_heap *h, void *mem); +/* Hand-calculated minimum heap sizes needed to return a successful + * 1-byte allocation. See details in lib/os/heap.[ch] + */ +#define Z_HEAP_MIN_SIZE (sizeof(void *) > 4 ? 56 : 44) + /** * @brief Define a static k_heap * @@ -5009,15 +5014,20 @@ void k_heap_free(struct k_heap *h, void *mem); * k_heap of the requested size. After kernel start, &name can be * used as if k_heap_init() had been called. * + * Note that this macro enforces a minimum size on the memory region + * to accommodate metadata requirements. Very small heaps will be + * padded to fit. + * * @param name Symbol name for the struct k_heap object * @param bytes Size of memory region, in bytes */ #define K_HEAP_DEFINE(name, bytes) \ - char __aligned(sizeof(void *)) kheap_##name[bytes]; \ + char __aligned(8) /* CHUNK_UNIT */ \ + kheap_##name[MAX(bytes, Z_HEAP_MIN_SIZE)]; \ Z_STRUCT_SECTION_ITERABLE(k_heap, name) = { \ .heap = { \ .init_mem = kheap_##name, \ - .init_bytes = (bytes), \ + .init_bytes = MAX(bytes, Z_HEAP_MIN_SIZE), \ }, \ } diff --git a/lib/os/heap.c b/lib/os/heap.c index 2dee196404c..ba1e9629c4a 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -403,7 +403,7 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) chunksz_t chunk0_size = chunksz(sizeof(struct z_heap) + nb_buckets * sizeof(struct z_heap_bucket)); - __ASSERT(chunk0_size + min_chunk_size(h) < heap_sz, "heap size is too small"); + __ASSERT(chunk0_size + min_chunk_size(h) <= heap_sz, "heap size is too small"); for (int i = 0; i < nb_buckets; i++) { h->buckets[i].next = 0;