Linux kmalloc doesn't return NULL on 0-byte allocations, but returns a
fixed ZERO_PSIZE_PTR constant, which triggers a NULL page exception when
dereferenced.

However, in barebox, allocator behavior differs:

 - glibc returns an unique pointer, but the standard also allows
   libc implementations to return NULL
 - tlsf returns NULL
 - dlmalloc returns a unique pointer
 - dummy malloc returns a unique pointer

Let's unify this discrepancy by doing something completely else and
returning a non-NULL non-unique pointer constant like Linux does.

The benefit of that is:

  - NULL return value always means out-of-memory
  - Accessing a zero sized allocation triggers a NULL page exception

This was noticed during porting of 9p2000 file system from Linux, which
does variable sized allocations and can end up allocating 0 bytes, but
expects that the allocation does not return NULL.

Signed-off-by: Ahmad Fatoum <[email protected]>
---
 arch/sandbox/os/libc_malloc.c | 25 ++++++++++++++++++++++++-
 common/calloc.c               |  2 +-
 common/dlmalloc.c             | 16 +++++++++-------
 common/dummy_malloc.c         |  3 +++
 common/tlsf.c                 | 15 ++++++++++-----
 common/tlsf_malloc.c          |  9 ---------
 include/malloc.h              | 13 +++++++++++++
 test/self/malloc.c            |  3 ++-
 8 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/arch/sandbox/os/libc_malloc.c b/arch/sandbox/os/libc_malloc.c
index dd0f8670ff71..bb4fb1c9ead4 100644
--- a/arch/sandbox/os/libc_malloc.c
+++ b/arch/sandbox/os/libc_malloc.c
@@ -6,6 +6,10 @@
 #include <stdlib.h>
 #include <malloc.h>
 
+#define ZERO_SIZE_PTR ((void *)16)
+
+#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
+                               (unsigned long)ZERO_SIZE_PTR)
 #define BAREBOX_ENOMEM 12
 #define BAREBOX_MALLOC_MAX_SIZE 0x40000000
 
@@ -19,6 +23,9 @@ void *barebox_memalign(size_t alignment, size_t bytes)
 {
        void *mem = NULL;
 
+       if (!bytes)
+               return ZERO_SIZE_PTR;
+
        if (alignment <= BAREBOX_MALLOC_MAX_SIZE && bytes <= 
BAREBOX_MALLOC_MAX_SIZE)
                mem = memalign(alignment, bytes);
        if (!mem)
@@ -29,9 +36,11 @@ void *barebox_memalign(size_t alignment, size_t bytes)
 
 void *barebox_malloc(size_t size)
 {
-
        void *mem = NULL;
 
+       if (!size)
+               return ZERO_SIZE_PTR;
+
        if (size <= BAREBOX_MALLOC_MAX_SIZE)
                mem = malloc(size);
        if (!mem)
@@ -42,11 +51,15 @@ void *barebox_malloc(size_t size)
 
 size_t barebox_malloc_usable_size(void *mem)
 {
+       if (ZERO_OR_NULL_PTR(mem))
+               return 0;
        return malloc_usable_size(mem);
 }
 
 void barebox_free(void *ptr)
 {
+       if (ZERO_OR_NULL_PTR(ptr))
+               return;
        free(ptr);
 }
 
@@ -54,6 +67,13 @@ void *barebox_realloc(void *ptr, size_t size)
 {
        void *mem = NULL;
 
+       if (!size) {
+               barebox_free(ptr);
+               return ZERO_SIZE_PTR;
+       }
+       if (ZERO_OR_NULL_PTR(ptr))
+               ptr = NULL;
+
        if (size <= BAREBOX_MALLOC_MAX_SIZE)
                mem = realloc(ptr, size);
        if (!mem)
@@ -67,6 +87,9 @@ void *barebox_calloc(size_t n, size_t elem_size)
        size_t product;
        void *mem = NULL;
 
+       if (!n || !elem_size)
+               return ZERO_SIZE_PTR;
+
        if (!__builtin_mul_overflow(n, elem_size, &product) &&
            product <= BAREBOX_MALLOC_MAX_SIZE)
                mem = calloc(n, elem_size);
diff --git a/common/calloc.c b/common/calloc.c
index 17cbd9beefee..ee9322509751 100644
--- a/common/calloc.c
+++ b/common/calloc.c
@@ -13,7 +13,7 @@ void *calloc(size_t n, size_t elem_size)
        size_t size = size_mul(elem_size, n);
        void *r = malloc(size);
 
-       if (r && !want_init_on_alloc())
+       if (!ZERO_OR_NULL_PTR(r) && !want_init_on_alloc())
                memset(r, 0x0, size);
 
        return r;
diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index 2b5723e127c4..c8e05ac68f2a 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -389,7 +389,7 @@
   returns a unique pointer for malloc(0), so does realloc(p, 0).
 */
 
-/*   #define REALLOC_ZERO_BYTES_FREES */
+#define REALLOC_ZERO_BYTES_FREES
 
 /*
   Define HAVE_MMAP to optionally make malloc() use mmap() to
@@ -1165,6 +1165,8 @@ void *dlmalloc(size_t bytes)
        if (bytes > MALLOC_MAX_SIZE) {
                errno = ENOMEM;
                return NULL;
+       } else if (bytes == 0) {
+               return ZERO_SIZE_PTR;
        }
 
        nb = request2size(bytes); /* padded request size; */
@@ -1363,7 +1365,7 @@ void dlfree(void *mem)
        mchunkptr fwd;          /* misc temp for linking */
        int islr;               /* track whether merging with last_remainder */
 
-       if (!mem)               /* free(0) has no effect */
+       if (ZERO_OR_NULL_PTR(mem))              /* free(0) has no effect */
                return;
 
        p = mem2chunk(mem);
@@ -1431,7 +1433,7 @@ size_t dlmalloc_usable_size(void *mem)
 {
        mchunkptr p;
 
-       if (!mem)
+       if (ZERO_OR_NULL_PTR(mem))
                return 0;
 
        p = mem2chunk(mem);
@@ -1495,7 +1497,7 @@ void *dlrealloc(void *oldmem, size_t bytes)
 #ifdef REALLOC_ZERO_BYTES_FREES
        if (bytes == 0) {
                dlfree(oldmem);
-               return NULL;
+               return ZERO_SIZE_PTR;
        }
 #endif
 
@@ -1505,7 +1507,7 @@ void *dlrealloc(void *oldmem, size_t bytes)
        }
 
        /* realloc of null is supposed to be same as malloc */
-       if (!oldmem)
+       if (ZERO_OR_NULL_PTR(oldmem))
                return dlmalloc(bytes);
 
        newp = oldp = mem2chunk(oldmem);
@@ -1760,8 +1762,8 @@ void *dlcalloc(size_t n, size_t elem_size)
 
        mem = dlmalloc(sz);
 
-       if (!mem)
-               return NULL;
+       if (ZERO_OR_NULL_PTR(mem))
+               return mem;
        else {
                p = mem2chunk(mem);
 
diff --git a/common/dummy_malloc.c b/common/dummy_malloc.c
index 4973f35650ab..956a54da0396 100644
--- a/common/dummy_malloc.c
+++ b/common/dummy_malloc.c
@@ -13,6 +13,9 @@ void *memalign(size_t alignment, size_t bytes)
 {
        void *mem = NULL;
 
+       if (!bytes)
+               return ZERO_SIZE_PTR;
+
        if (alignment <= MALLOC_MAX_SIZE && bytes <= MALLOC_MAX_SIZE)
                mem = sbrk(bytes + alignment);
        if (!mem) {
diff --git a/common/tlsf.c b/common/tlsf.c
index 146f1b7ebe81..5504453a9453 100644
--- a/common/tlsf.c
+++ b/common/tlsf.c
@@ -743,7 +743,7 @@ void tlsf_walk_pool(pool_t pool, tlsf_walker walker, void* 
user)
 size_t tlsf_block_size(void* ptr)
 {
        size_t size = 0;
-       if (ptr)
+       if (likely(!ZERO_OR_NULL_PTR(ptr)))
        {
                const block_header_t* block = block_from_ptr(ptr);
                size = block_size(block);
@@ -935,6 +935,8 @@ void* tlsf_malloc(tlsf_t tlsf, size_t size)
        const size_t adjust = adjust_request_size(size, ALIGN_SIZE);
        block_header_t* block;
 
+       if (!size)
+               return ZERO_SIZE_PTR;
        if (!adjust)
                return NULL;
 
@@ -961,12 +963,14 @@ void* tlsf_memalign(tlsf_t tlsf, size_t align, size_t 
size)
 
        /*
        ** If alignment is less than or equals base alignment, we're done.
-       ** If we requested 0 bytes, return null, as tlsf_malloc(0) does.
+       ** If we requested 0 bytes, return ZERO_SIZE_PTR, as tlsf_malloc(0) 
does.
        */
        const size_t aligned_size = (adjust && align > ALIGN_SIZE) ? 
size_with_gap : adjust;
 
        block_header_t* block;
 
+       if (!size)
+               return ZERO_SIZE_PTR;
        if (!adjust || !size_with_gap)
                return NULL;
 
@@ -1008,7 +1012,7 @@ void* tlsf_memalign(tlsf_t tlsf, size_t align, size_t 
size)
 void tlsf_free(tlsf_t tlsf, void* ptr)
 {
        /* Don't attempt to free a NULL pointer. */
-       if (ptr)
+       if (!ZERO_OR_NULL_PTR(ptr))
        {
                control_t* control = tlsf_cast(control_t*, tlsf);
                block_header_t* block = block_from_ptr(ptr);
@@ -1042,12 +1046,13 @@ void* tlsf_realloc(tlsf_t tlsf, void* ptr, size_t size)
        void* p = 0;
 
        /* Zero-size requests are treated as free. */
-       if (ptr && size == 0)
+       if (size == 0)
        {
                tlsf_free(tlsf, ptr);
+               return ZERO_SIZE_PTR;
        }
        /* Requests with NULL pointers are treated as malloc. */
-       else if (!ptr)
+       else if (ZERO_OR_NULL_PTR(ptr))
        {
                p = tlsf_malloc(tlsf, size);
        }
diff --git a/common/tlsf_malloc.c b/common/tlsf_malloc.c
index 1b9d9c7702c2..6a90ee47199f 100644
--- a/common/tlsf_malloc.c
+++ b/common/tlsf_malloc.c
@@ -17,12 +17,6 @@ extern tlsf_t tlsf_mem_pool;
 void *malloc(size_t bytes)
 {
        void *mem;
-       /*
-        * tlsf_malloc returns NULL for zero bytes, we instead want
-        * to have a valid pointer.
-        */
-       if (!bytes)
-               bytes = 1;
 
        mem = tlsf_malloc(tlsf_mem_pool, bytes);
        if (!mem)
@@ -40,9 +34,6 @@ EXPORT_SYMBOL(free);
 
 size_t malloc_usable_size(void *mem)
 {
-       if (!mem)
-               return 0;
-
        return tlsf_block_size(mem);
 }
 EXPORT_SYMBOL(malloc_usable_size);
diff --git a/include/malloc.h b/include/malloc.h
index c38726342c08..0b74746360c0 100644
--- a/include/malloc.h
+++ b/include/malloc.h
@@ -8,6 +8,19 @@
 #define MALLOC_SHIFT_MAX       30
 #define MALLOC_MAX_SIZE                (1UL << MALLOC_SHIFT_MAX)
 
+/*
+ * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
+ *
+ * Dereferencing ZERO_SIZE_PTR will lead to a distinct access fault.
+ *
+ * ZERO_SIZE_PTR can be passed to free though in the same way that NULL can.
+ * Both make free a no-op.
+ */
+#define ZERO_SIZE_PTR ((void *)16)
+
+#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
+                               (unsigned long)ZERO_SIZE_PTR)
+
 #if IN_PROPER
 void *malloc(size_t) __alloc_size(1);
 size_t malloc_usable_size(void *);
diff --git a/test/self/malloc.c b/test/self/malloc.c
index 79a32b5d97c7..52f9fc344c1a 100644
--- a/test/self/malloc.c
+++ b/test/self/malloc.c
@@ -122,7 +122,8 @@ static void test_malloc(void)
        p = expect_alloc_ok(malloc(0));
        tmp = expect_alloc_ok(malloc(0));
 
-       __expect_cond(p != tmp, true, "allocate distinct 0-size buffers", 
__func__, __LINE__);
+       __expect_cond(p == tmp, true, "allocate equal 0-size buffers", 
__func__, __LINE__);
+       __expect_cond(p == ZERO_SIZE_PTR, true, "get ZERO_SIZE_PTR for 0-size 
buffers", __func__, __LINE__);
 
        free(p);
        free(tmp);
-- 
2.39.5


Reply via email to