Skip to content

Commit

Permalink
Fix, calloc now fails on extra-large request. (#8482)
Browse files Browse the repository at this point in the history
Added code to handle multiply overflow in calloc.
Added code to handle add overflow in umm_poison_*
  • Loading branch information
mhightower83 committed Feb 12, 2022
1 parent f60defc commit e6fc76a
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 18 deletions.
17 changes: 12 additions & 5 deletions cores/esp8266/heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <stdlib.h>
#include "umm_malloc/umm_malloc.h"
extern "C" size_t umm_umul_sat(const size_t a, const size_t b);;

// Need FORCE_ALWAYS_INLINE to put HeapSelect class constructor/deconstructor in IRAM
#define FORCE_ALWAYS_INLINE_HEAP_SELECT
Expand Down Expand Up @@ -153,7 +154,7 @@ void* _calloc_r(struct _reent* unused, size_t count, size_t size)
{
(void) unused;
void *ret = calloc(count, size);
PTR_CHECK__LOG_LAST_FAIL(ret, count * size);
PTR_CHECK__LOG_LAST_FAIL(ret, umm_umul_sat(count, size));
return ret;
}

Expand Down Expand Up @@ -247,8 +248,11 @@ void* IRAM_ATTR calloc(size_t count, size_t size)
INTEGRITY_CHECK__ABORT();
POISON_CHECK__ABORT();
void* ret = UMM_CALLOC(count, size);
PTR_CHECK__LOG_LAST_FAIL(ret, count * size);
OOM_CHECK__PRINT_OOM(ret, size);
#if defined(DEBUG_ESP_OOM)
size_t total_size = umm_umul_sat(count, size);// For logging purposes
#endif
PTR_CHECK__LOG_LAST_FAIL(ret, total_size);
OOM_CHECK__PRINT_OOM(ret, total_size);
return ret;
}

Expand Down Expand Up @@ -287,8 +291,11 @@ void* IRAM_ATTR heap_pvPortCalloc(size_t count, size_t size, const char* file, i
INTEGRITY_CHECK__PANIC_FL(file, line);
POISON_CHECK__PANIC_FL(file, line);
void* ret = UMM_CALLOC(count, size);
PTR_CHECK__LOG_LAST_FAIL_FL(ret, count * size, file, line);
OOM_CHECK__PRINT_LOC(ret, size, file, line);
#if defined(DEBUG_ESP_OOM)
size_t total_size = umm_umul_sat(count, size);
#endif
PTR_CHECK__LOG_LAST_FAIL_FL(ret, total_size, file, line);
OOM_CHECK__PRINT_LOC(ret, total_size, file, line);
return ret;
}

Expand Down
30 changes: 29 additions & 1 deletion cores/esp8266/umm_malloc/umm_local.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void *umm_poison_realloc_fl(void *ptr, size_t size, const char *file, int line)

ptr = get_unpoisoned_check_neighbors(ptr, file, line);

size += poison_size(size);
add_poison_size(&size);
ret = umm_realloc(ptr, size);

ret = get_poisoned(ret, size);
Expand Down Expand Up @@ -296,4 +296,32 @@ size_t ICACHE_FLASH_ATTR umm_get_free_null_count(void) {
}
#endif // UMM_STATS_FULL

#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
/*
* Saturated unsigned add
* Poison added to allocation size requires overflow protection.
*/
static size_t umm_uadd_sat(const size_t a, const size_t b) {
size_t r = a + b;
if (r < a) {
return SIZE_MAX;
}
return r;
}
#endif

/*
* Use platform-specific functions to protect against unsigned overflow/wrap by
* implementing saturated unsigned multiply.
* The function umm_calloc requires a saturated multiply function.
*/
size_t umm_umul_sat(const size_t a, const size_t b) {
size_t r;
if (__builtin_mul_overflow(a, b, &r)) {
return SIZE_MAX;
}
return r;
}


#endif // BUILD_UMM_MALLOC_C
8 changes: 8 additions & 0 deletions cores/esp8266/umm_malloc/umm_local.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
#define memset ets_memset


/*
* Saturated unsigned add and unsigned multiply
*/
size_t umm_umul_sat(const size_t a, const size_t b); // share with heap.cpp
#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
static size_t umm_uadd_sat(const size_t a, const size_t b);
#endif

/*
* This redefines DBGLOG_FORCE defined in dbglog/dbglog.h
* Just for printing from umm_info() which is assumed to always be called from
Expand Down
8 changes: 6 additions & 2 deletions cores/esp8266/umm_malloc/umm_malloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1214,10 +1214,14 @@ void *umm_realloc(void *ptr, size_t size) {
void *umm_calloc(size_t num, size_t item_size) {
void *ret;

ret = umm_malloc((size_t)(item_size * num));
// Use saturated multiply.
// Rely on umm_malloc to supply the fail response as needed.
size_t size = umm_umul_sat(num, item_size);

ret = umm_malloc(size);

if (ret) {
memset(ret, 0x00, (size_t)(item_size * num));
memset(ret, 0x00, size);
}

return ret;
Expand Down
27 changes: 17 additions & 10 deletions cores/esp8266/umm_malloc/umm_poison.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,19 @@
#include <stddef.h>
#include <stdbool.h>

#define UMM_POISON_BLOCK_SIZE (UMM_POISON_SIZE_BEFORE + sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_AFTER)

/*
* Yields a size of the poison for the block of size `s`.
* Yields the total size of a poison block of size `s`.
* If `s` is 0, returns 0.
* If result overflows/wraps, return saturation value.
*/
static size_t poison_size(size_t s) {
return s ? (UMM_POISON_SIZE_BEFORE +
sizeof(UMM_POISONED_BLOCK_LEN_TYPE) +
UMM_POISON_SIZE_AFTER)
: 0;
static void add_poison_size(size_t* s) {
if (*s == 0) {
return;
}

*s = umm_uadd_sat(*s, UMM_POISON_BLOCK_SIZE);
}

/*
Expand Down Expand Up @@ -158,7 +162,7 @@ static void *get_unpoisoned(void *vptr) {
void *umm_poison_malloc(size_t size) {
void *ret;

size += poison_size(size);
add_poison_size(&size);

ret = umm_malloc(size);

Expand All @@ -171,9 +175,12 @@ void *umm_poison_malloc(size_t size) {

void *umm_poison_calloc(size_t num, size_t item_size) {
void *ret;
size_t size = item_size * num;

size += poison_size(size);
// Use saturated multiply.
// Rely on umm_malloc to supply the fail response as needed.
size_t size = umm_umul_sat(num, item_size);

add_poison_size(&size);

ret = umm_malloc(size);

Expand All @@ -193,7 +200,7 @@ void *umm_poison_realloc(void *ptr, size_t size) {

ptr = get_unpoisoned(ptr);

size += poison_size(size);
add_poison_size(&size);
ret = umm_realloc(ptr, size);

ret = get_poisoned(ret, size);
Expand Down

0 comments on commit e6fc76a

Please sign in to comment.