[Minios-devel] [UNIKRAFT PATCH v5 2/4] lib/ukalloc: fix multiple issues in uk_posix_memalign_ifpages

[Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>]

uk_posix_memalign_ifpages might return out-of-bounds pointers when
alignment is smaller than  __PAGE_SIZE (for example align = __PAGE_SIZE /
2, and size = __PAGE_SIZE - sizeof(size_t)).

Address this issue by reserving at least align bytes for the page number
information. This introduces the need to store not only page number
information in the metadata, but also a base pointer since we will now have
potentially arbitrary offsets within allocated boundaries.

This also adds support for align > __PAGE_SIZE.

Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>

diff --git a/lib/ukalloc/alloc.c b/lib/ukalloc/alloc.c
index a57a13c..124f411 100644
--- a/lib/ukalloc/alloc.c
+++ b/lib/ukalloc/alloc.c
@@ -112,9 +112,15 @@ int uk_alloc_set_default(struct uk_alloc *a)
 }
 
 #if CONFIG_LIBUKALLOC_IFPAGES
-static void *uk_get_real_start(const void *ptr)
+
+struct metadata_ifpages {
+       unsigned long   num_pages;
+       void*           base;
+};
+
+static struct metadata_ifpages *uk_get_metadata(const void *ptr)
 {
-       void *intptr;
+       struct metadata_ifpages *metadata;
 
        /* a ptr less or equal to page size
         * would mean that the actual allocated
@@ -122,46 +128,31 @@ static void *uk_get_real_start(const void *ptr)
         */
        UK_ASSERT((uintptr_t) ptr > __PAGE_SIZE);
 
-       intptr = (void *) ALIGN_DOWN((uintptr_t) ptr,
-                                    (uintptr_t) __PAGE_SIZE);
-       if (intptr == ptr) {
+       metadata = (struct metadata_ifpages*) ALIGN_DOWN((uintptr_t) ptr,
+                                                        (uintptr_t) 
__PAGE_SIZE);
+       if (metadata == ptr) {
                /* special case: the memory was page-aligned.
-                * In this case the size information lies at the start of the
+                * In this case the metadata lies at the start of the
                 * previous page, with the rest of that page unused.
                 */
-               intptr -= __PAGE_SIZE;
+               metadata -= __PAGE_SIZE;
        }
-       return intptr;
+
+       return metadata;
 }
 
 static size_t uk_getmallocsize(const void *ptr)
 {
-       size_t *intptr = uk_get_real_start(ptr);
-       size_t mallocsize = __PAGE_SIZE << (*intptr);
-
-       if (((uintptr_t) ptr & (~__PAGE_MASK)) == 0) {
-               /*
-                * special case: the memory was page-aligned
-                * In this case the allocated size should not account for the
-                * previous page which was used for storing the number of pages
-                */
-               mallocsize -= __PAGE_SIZE;
-       } else {
-               /*
-                * If pointer is not page aligned it means the header is
-                * on the same page. This will break if metadata size increases
-                */
-               mallocsize -= sizeof(*intptr);
-       }
-
-       return mallocsize;
+       struct metadata_ifpages *metadata = uk_get_metadata(ptr);
+       return metadata->base + (metadata->num_pages) * __PAGE_SIZE - ptr;
 }
 
 void *uk_malloc_ifpages(struct uk_alloc *a, size_t size)
 {
        uintptr_t intptr;
        unsigned long num_pages;
-       size_t realsize = sizeof(num_pages) + size;
+       struct metadata_ifpages *metadata;
+       size_t realsize = sizeof(*metadata) + size;
 
        UK_ASSERT(a);
        if (!size)
@@ -173,20 +164,23 @@ void *uk_malloc_ifpages(struct uk_alloc *a, size_t size)
        if (!intptr)
                return NULL;
 
-       *(size_t *)intptr = num_pages;
-       return (void *)(intptr + sizeof(num_pages));
+       metadata = (struct metadata_ifpages *) intptr;
+       metadata->num_pages = num_pages;
+       metadata->base = (void*) intptr;
+
+       return (void *)(intptr + sizeof(*metadata));
 }
 
 void uk_free_ifpages(struct uk_alloc *a, void *ptr)
 {
-       size_t *intptr;
+       struct metadata_ifpages *metadata;
 
        UK_ASSERT(a);
        if (!ptr)
                return;
 
-       intptr = uk_get_real_start(ptr);
-       uk_pfree(a, intptr, *intptr);
+       metadata = uk_get_metadata(ptr);
+       uk_pfree(a, metadata->base, metadata->num_pages);
 }
 
 void *uk_realloc_ifpages(struct uk_alloc *a, void *ptr, size_t size)
@@ -221,14 +215,14 @@ void *uk_realloc_ifpages(struct uk_alloc *a, void *ptr, 
size_t size)
 int uk_posix_memalign_ifpages(struct uk_alloc *a,
                                void **memptr, size_t align, size_t size)
 {
-       uintptr_t *intptr;
-       size_t realsize;
+       struct metadata_ifpages *metadata;
+       size_t realsize, metadata_offset;
        unsigned long num_pages;
+       uintptr_t *intptr;
 
        UK_ASSERT(a);
        if (((align - 1) & align) != 0
-           || (align % sizeof(void *)) != 0
-           || (align > __PAGE_SIZE))
+           || (align % sizeof(void *)) != 0)
                return EINVAL;
 
        if (!size) {
@@ -236,14 +230,12 @@ int uk_posix_memalign_ifpages(struct uk_alloc *a,
                return EINVAL;
        }
 
-       /* For page-aligned memory blocks, the size information is not stored
-        * immediately preceding the memory block, but instead at the
-        * beginning of the page preceeding the memory handed out via malloc.
+       /* Store size information preceeding the memory block. Since we return
+        * pointers aligned at `align` we need to reserve at least that much
+        * space for the size information.
         */
-       if (align == __PAGE_SIZE)
-               realsize = ALIGN_UP(size + __PAGE_SIZE, align);
-       else
-               realsize = ALIGN_UP(size + sizeof(num_pages), align);
+       metadata_offset = align > sizeof(*metadata) ? align : sizeof(*metadata);
+       realsize = ALIGN_UP(size + metadata_offset, align);
 
        num_pages = size_to_num_pages(realsize);
        intptr = uk_palloc(a, num_pages);
@@ -251,8 +243,12 @@ int uk_posix_memalign_ifpages(struct uk_alloc *a,
        if (!intptr)
                return ENOMEM;
 
-       *(size_t *)intptr = num_pages;
-       *memptr = (void *) ALIGN_UP((uintptr_t)intptr + sizeof(num_pages), 
align);
+       *memptr = (void *) ALIGN_UP((uintptr_t)intptr + metadata_offset, align);
+
+       metadata = uk_get_metadata(*memptr);
+       metadata->num_pages = num_pages;
+       metadata->base = intptr;
+
        return 0;
 }
 
-- 
2.7.4


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel