Skip to content

Commit 36de71f

Browse files
mhightower83hasenradball
authored andcommitted
Fixes occasional UMM_POISON failure (esp8266#8953)
* Fixes occasional UMM_POISON failure Bug introduced with PR fix esp8266#8914. When a reallocated pointer could not grow in place, a replacement allocation was created. Then UMM_POISON was written to the wrong block. * Fix umm_poison data corruption on realloc when memory move is used. Bug introduced with PR fix esp8266#8914 * refactored to resolve unused error in some build contexts
1 parent 54c465e commit 36de71f

File tree

2 files changed

+22
-14
lines changed

2 files changed

+22
-14
lines changed

cores/esp8266/umm_malloc/umm_malloc.cpp

+9-7
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ void *umm_realloc(void *ptr, size_t size) {
928928

929929
uint16_t c;
930930

931-
size_t curSize;
931+
[[maybe_unused]] size_t curSize;
932932

933933
UMM_CHECK_INITIALIZED();
934934

@@ -1089,7 +1089,8 @@ void *umm_realloc(void *ptr, size_t size) {
10891089
STATS__FREE_BLOCKS_UPDATE(-prevBlockSize);
10901090
STATS__FREE_BLOCKS_ISR_MIN();
10911091
blockSize += prevBlockSize;
1092-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size); // Fix allocation so ISR poison check is good
1092+
// Fix new allocation such that poison checks from an ISR pass.
1093+
POISON_CHECK_SET_POISON_BLOCKS((void *)&UMM_DATA(c), blockSize);
10931094
UMM_CRITICAL_SUSPEND(id_realloc);
10941095
UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
10951096
ptr = (void *)&UMM_DATA(c);
@@ -1111,7 +1112,7 @@ void *umm_realloc(void *ptr, size_t size) {
11111112
#else
11121113
blockSize += (prevBlockSize + nextBlockSize);
11131114
#endif
1114-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
1115+
POISON_CHECK_SET_POISON_BLOCKS((void *)&UMM_DATA(c), blockSize);
11151116
UMM_CRITICAL_SUSPEND(id_realloc);
11161117
UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
11171118
ptr = (void *)&UMM_DATA(c);
@@ -1123,7 +1124,7 @@ void *umm_realloc(void *ptr, size_t size) {
11231124
void *oldptr = ptr;
11241125
if ((ptr = umm_malloc_core(_context, size))) {
11251126
DBGLOG_DEBUG("realloc %i to a bigger block %i, copy, and free the old\n", blockSize, blocks);
1126-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
1127+
(void)POISON_CHECK_SET_POISON(ptr, size);
11271128
UMM_CRITICAL_SUSPEND(id_realloc);
11281129
UMM_POISON_MEMCPY(ptr, oldptr, curSize);
11291130
UMM_CRITICAL_RESUME(id_realloc);
@@ -1186,7 +1187,8 @@ void *umm_realloc(void *ptr, size_t size) {
11861187
blockSize = blocks;
11871188
#endif
11881189
}
1189-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
1190+
// Fix new allocation such that poison checks from an ISR pass.
1191+
POISON_CHECK_SET_POISON_BLOCKS((void *)&UMM_DATA(c), blockSize);
11901192
UMM_CRITICAL_SUSPEND(id_realloc);
11911193
UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
11921194
ptr = (void *)&UMM_DATA(c);
@@ -1204,7 +1206,7 @@ void *umm_realloc(void *ptr, size_t size) {
12041206
void *oldptr = ptr;
12051207
if ((ptr = umm_malloc_core(_context, size))) {
12061208
DBGLOG_DEBUG("realloc %d to a bigger block %d, copy, and free the old\n", blockSize, blocks);
1207-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
1209+
(void)POISON_CHECK_SET_POISON(ptr, size);
12081210
UMM_CRITICAL_SUSPEND(id_realloc);
12091211
UMM_POISON_MEMCPY(ptr, oldptr, curSize);
12101212
UMM_CRITICAL_RESUME(id_realloc);
@@ -1230,7 +1232,7 @@ void *umm_realloc(void *ptr, size_t size) {
12301232
void *oldptr = ptr;
12311233
if ((ptr = umm_malloc_core(_context, size))) {
12321234
DBGLOG_DEBUG("realloc %d to a bigger block %d, copy, and free the old\n", blockSize, blocks);
1233-
POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
1235+
(void)POISON_CHECK_SET_POISON(ptr, size);
12341236
UMM_CRITICAL_SUSPEND(id_realloc);
12351237
UMM_POISON_MEMCPY(ptr, oldptr, curSize);
12361238
UMM_CRITICAL_RESUME(id_realloc);

cores/esp8266/umm_malloc/umm_malloc_cfg.h

+13-7
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,16 @@ extern bool umm_poison_check(void);
619619
void *umm_poison_realloc_fl(void *ptr, size_t size, const char *file, int line);
620620
void umm_poison_free_fl(void *ptr, const char *file, int line);
621621
#define POISON_CHECK_SET_POISON(p, s) get_poisoned(p, s)
622-
#define UMM_POISON_SKETCH_PTR(p) ((void*)((uintptr_t)p + sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_BEFORE))
623-
#define UMM_POISON_SKETCH_PTRSZ(s) (s - sizeof(UMM_POISONED_BLOCK_LEN_TYPE) - UMM_POISON_SIZE_BEFORE - UMM_POISON_SIZE_AFTER)
622+
#define POISON_CHECK_SET_POISON_BLOCKS(p, s) \
623+
do { \
624+
size_t super_size = (s * sizeof(umm_block)) - (sizeof(((umm_block *)0)->header)); \
625+
get_poisoned(p, super_size); \
626+
} while (false)
627+
#define UMM_POISON_SKETCH_PTR(p) ((void *)((uintptr_t)p + sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_BEFORE))
628+
#define UMM_POISON_SKETCH_PTRSZ(p) (*(UMM_POISONED_BLOCK_LEN_TYPE *)p)
629+
#define UMM_POISON_MEMMOVE(t, p, s) memmove(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(p))
630+
#define UMM_POISON_MEMCPY(t, p, s) memcpy(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(p))
631+
624632
#if defined(UMM_POISON_CHECK_LITE)
625633
/*
626634
* We can safely do individual poison checks at free and realloc and stay
@@ -641,13 +649,11 @@ void umm_poison_free_fl(void *ptr, const char *file, int line);
641649
#define POISON_CHECK() 1
642650
#define POISON_CHECK_NEIGHBORS(c) do {} while (false)
643651
#define POISON_CHECK_SET_POISON(p, s) (p)
644-
#define UMM_POISON_SKETCH_PTR(p) (p)
645-
#define UMM_POISON_SKETCH_PTRSZ(s) (s)
652+
#define POISON_CHECK_SET_POISON_BLOCKS(p, s)
653+
#define UMM_POISON_MEMMOVE(t, p, s) memmove((t), (p), (s))
654+
#define UMM_POISON_MEMCPY(t, p, s) memcpy((t), (p), (s))
646655
#endif
647656

648-
#define UMM_POISON_MEMMOVE(t, p, s) memmove(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(s))
649-
#define UMM_POISON_MEMCPY(t, p, s) memcpy(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(s))
650-
651657
#if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
652658
/*
653659
* Overhead adjustments needed for free_blocks to express the number of bytes

0 commit comments

Comments
 (0)