From 2b38f7fd79836d1f15d1b57ea90932016641f659 Mon Sep 17 00:00:00 2001 From: Jerome Forissier Date: Fri, 4 May 2018 09:54:24 +0200 Subject: libutils: bget_malloc.c: fix overflow tests The overflow tests in raw_calloc(), raw_realloc() and raw_malloc() are wrong. They don't work as expected when hdr_size and ftr_size are both zero. The bug is exposed by commit 96c1d8c56cde ("ta: TEE_Malloc() and friend: skips layers") which causes xtest 8033 to fail because TEE_Malloc(0, TEE_MALLOC_FILL_ZERO) now returns NULL. In addition, the allocation functions in bget.c (bget(), bgetz() and bgetr()) take a parameter of type bufsize for the allocation size. This happens to be a (signed) long. On the other hand, raw_malloc(), raw_calloc() and raw_realloc() take a size_t parameter which is unsigned long. Therefore, large size values are incorrectly interpreted as being negative by the bget code, which then asserts. When run in the context of a TA, this causes a TA panic instead of a TEE_ERROR_OUT_OF_MEMORY error. This bug is also exposed by commit 96c1d8c56cde ("ta: TEE_Malloc() and friend: skips layers") and makes xtest 8034 and 8042 fail. 8034 is TEE_Malloc(0xFFFFFFFE, TEE_MALLOC_FILL_ZERO) while 8042 is TEE_Realloc(ptr, 0xFFFFFFFE). Rework the raw_calloc(), raw_realloc() and raw_malloc() functions to use the MUL_OVERFLOW() and ADD_OVERFLOW() macros instead, operating on the proper types. Signed-off-by: Jerome Forissier Reviewed-by: Jens Wiklander Tested-by: Jerome Forissier --- lib/libutils/isoc/bget_malloc.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'lib/libutils/isoc') diff --git a/lib/libutils/isoc/bget_malloc.c b/lib/libutils/isoc/bget_malloc.c index 477cf42f..9aaf9b97 100644 --- a/lib/libutils/isoc/bget_malloc.c +++ b/lib/libutils/isoc/bget_malloc.c @@ -312,7 +312,7 @@ static void *raw_malloc(size_t hdr_size, size_t ftr_size, size_t pl_size, struct bpoolset *poolset) { void *ptr = NULL; - size_t s = hdr_size + ftr_size + pl_size; + bufsize s; /* * Make sure that malloc has correct alignment of returned buffers. @@ -323,8 +323,10 @@ static void *raw_malloc(size_t hdr_size, size_t ftr_size, size_t pl_size, raw_malloc_validate_pools(); - /* Check wrapping */ - if (s < pl_size) + /* Compute total size */ + if (ADD_OVERFLOW(pl_size, hdr_size, &s)) + goto out; + if (ADD_OVERFLOW(s, ftr_size, &s)) goto out; /* BGET doesn't like 0 sized allocations */ @@ -349,13 +351,17 @@ static void raw_free(void *ptr, struct bpoolset *poolset) static void *raw_calloc(size_t hdr_size, size_t ftr_size, size_t pl_nmemb, size_t pl_size, struct bpoolset *poolset) { - size_t s = hdr_size + ftr_size + pl_nmemb * pl_size; void *ptr = NULL; + bufsize s; raw_malloc_validate_pools(); - /* Check wrapping */ - if (s < pl_nmemb || s < pl_size) + /* Compute total size */ + if (MUL_OVERFLOW(pl_nmemb, pl_size, &s)) + goto out; + if (ADD_OVERFLOW(s, hdr_size, &s)) + goto out; + if (ADD_OVERFLOW(s, ftr_size, &s)) goto out; /* BGET doesn't like 0 sized allocations */ @@ -372,11 +378,13 @@ out: static void *raw_realloc(void *ptr, size_t hdr_size, size_t ftr_size, size_t pl_size, struct bpoolset *poolset) { - size_t s = hdr_size + ftr_size + pl_size; void *p = NULL; + bufsize s; - /* Check wrapping */ - if (s < pl_size) + /* Compute total size */ + if (ADD_OVERFLOW(pl_size, hdr_size, &s)) + goto out; + if (ADD_OVERFLOW(s, ftr_size, &s)) goto out; raw_malloc_validate_pools(); -- cgit v1.2.3