Skip to content

Commit 4a0b66b

Browse files
authored
Heap addendum to handle changes in NON-OS SDK 3.0.x (#8746)
## WPA2 Enterprise connections References - merged PRs: * #8529 * #8566 - these occurred with connect/disconnect with WPA-Enterprise * #8736 (comment) The NON-OS SDK 3.0.x has breaking changes to the [`pvPortMalloc`](https://github.com/espressif/ESP8266_NONOS_SDK/blob/bf890b22e57a41d5cda00f9c8191f3f7035a87b4/include/mem.h#L42) function. They added a new `bool` argument for selecting a heap. ```cpp void *pvPortMalloc (size_t sz, const char *, unsigned, bool); ``` To avoid breaking the build, I added a new thin wrapper function `sdk3_pvPortMalloc` to `heap.cpp`. Edited new SDK LIBs to call `pvPortMalloc`'s replacement `sdk3_pvPortMalloc`. They also added `pvPortZallocIram` and `pvPortCallocIram`, which are not a problem to support. Support added to `heap.cpp`. Issues with WPA2 Enterprise in new SDKs: * v3.0.0 and v3.0.1 - have the same memory leak and duplicate free bugs from before * v3.0.2 through v3.0.5 - have the same memory leak; however, _no_ duplicate free crash. * memory leak can be seen by cycling through setup, connect, disconnect, and clear setup - repeatedly. Updated `wpa2_eap_patch.cpp` and binary patch scripts to handle v3.0.0 through v3.0.5. Patched SDKs v3.0.0 through v3.0.5 ## Duplicate Non-32-bit exception handler Issue: At v3.0.0 and above `libmain.a` supplies a built-in exception handler (`load_non_32_wide_handler`) for non-32-bit access. Our non-32-bit access handler (`non32xfer_exception_handler`) overrides it. Solution: Add "weak" attribute to symbol `load_non_32_wide_handler`. Adjust the build to default to the SDK's built-in non-32-bit handler. If there is a need to use our non-32-bit handler, make the selection from the Arduino IDE Tools menu `Non-32-Bit Access: "Byte/Word access to IRAM/PROGMEM (very slow)"`. With SDKs v3.0.0 and above a "non-32-bit exception handler" is always present.
1 parent d3c8d27 commit 4a0b66b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+317
-66
lines changed

cores/esp8266/core_esp8266_main.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ extern "C" void user_init(void) {
496496
install_vm_exception_handler();
497497
#endif
498498

499-
#if defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP)
499+
#if defined(NON32XFER_HANDLER) || (defined(MMU_IRAM_HEAP) && (NONOSDK < (0x30000 - 1)))
500500
install_non32xfer_exception_handler();
501501
#endif
502502

cores/esp8266/core_esp8266_non32xfer.cpp

+29-2
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,14 @@
2626
* Code taken directly from @pvvx's public domain code in
2727
* https://github.com/pvvx/esp8266web/blob/master/app/sdklib/system/app_main.c
2828
*
29+
* In Espressif versions NONOSDK v3.0.0+ a similar feature was added
30+
* load_non_32_wide_handler. Theirs is always loaded. Add weak attribute to
31+
* theirs so we can choose by adding an alias to ours.
2932
*
3033
*/
3134

3235
#include <Arduino.h>
36+
#include <user_interface.h>
3337
#define VERIFY_C_ASM_EXCEPTION_FRAME_STRUCTURE
3438
#include <esp8266_undocumented.h>
3539
#include <core_esp8266_non32xfer.h>
@@ -58,10 +62,13 @@ extern "C" {
5862

5963
#define EXCCAUSE_LOAD_STORE_ERROR 3 /* Non 32-bit read/write error */
6064

65+
#if (defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP)) && (NONOSDK < (0x30000 - 1))
6166
static fn_c_exception_handler_t old_c_handler = NULL;
67+
#endif
6268

69+
#if defined(NON32XFER_HANDLER) || (defined(MMU_IRAM_HEAP) && (NONOSDK < (0x30000 - 1)))
6370
static
64-
IRAM_ATTR void non32xfer_exception_handler(struct __exception_frame *ef, int cause)
71+
IRAM_ATTR void non32xfer_exception_handler(struct __exception_frame *ef, [[maybe_unused]] int cause)
6572
{
6673
do {
6774
uint32_t insn, excvaddr;
@@ -138,6 +145,7 @@ IRAM_ATTR void non32xfer_exception_handler(struct __exception_frame *ef, int cau
138145
} while(false);
139146

140147
/* Fail request, die */
148+
#if (NONOSDK < (0x30000 - 1))
141149
/*
142150
The old handler points to the SDK. Be alert for HWDT when Calling with
143151
INTLEVEL != 0. I cannot create it any more. I thought I saw this as a
@@ -148,16 +156,23 @@ IRAM_ATTR void non32xfer_exception_handler(struct __exception_frame *ef, int cau
148156
old_c_handler(ef, cause);
149157
return;
150158
}
151-
159+
#endif
152160
/*
153161
Calling _xtos_unhandled_exception(ef, cause) in the Boot ROM, gets us a
154162
hardware wdt.
155163
156164
Use panic instead as a fall back. It will produce a stack trace.
157165
*/
166+
#if defined(DEBUG_ESP_PORT) || defined(DEBUG_ESP_MMU)
158167
panic();
168+
#else
169+
// For non-debug builds, save on resources
170+
abort();
171+
#endif
159172
}
173+
#endif // #if defined(NON32XFER_HANDLER) || (defined(MMU_IRAM_HEAP) && (NONOSDK < (0x30000 - 1)))
160174

175+
#if (defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP)) && (NONOSDK < (0x30000 - 1))
161176
/*
162177
To operate reliably, this module requires the new
163178
`_xtos_set_exception_handler` from `exc-sethandler.cpp` and
@@ -174,5 +189,17 @@ void install_non32xfer_exception_handler(void) {
174189
non32xfer_exception_handler);
175190
}
176191
}
192+
#else
193+
// For v3.0.x SDKs, no need for install - call_user_start will do the job.
194+
// Need this for build dependencies
195+
void install_non32xfer_exception_handler(void) __attribute__((weak));
196+
void install_non32xfer_exception_handler(void) {}
197+
#endif
198+
199+
#if defined(NON32XFER_HANDLER)
200+
// For SDKs 3.0.x, we made load_non_32_wide_handler in
201+
// libmain.c:user_exceptions.o a weak symbol allowing this override to work.
202+
extern void load_non_32_wide_handler(struct __exception_frame *ef, int cause) __attribute__((alias("non32xfer_exception_handler")));
203+
#endif
177204

178205
};

cores/esp8266/exc-sethandler.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@
3939
* architecture, I am not convinced it can be done safely.
4040
*
4141
*/
42+
#include <user_interface.h> // need NONOSDK
4243

43-
#if defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP) || defined(NEW_EXC_C_WRAPPER) || defined(MMU_EXTERNAL_HEAP)
44+
#if defined(NON32XFER_HANDLER) || defined(MMU_IRAM_HEAP) || \
45+
defined(NEW_EXC_C_WRAPPER) || defined(MMU_EXTERNAL_HEAP) || (NONOSDK >= (0x30000 - 1))
4446

4547
/*
4648
* The original module source code came from:

cores/esp8266/heap.cpp

+60-5
Original file line numberDiff line numberDiff line change
@@ -356,25 +356,25 @@ void system_show_malloc(void)
356356
void* IRAM_ATTR pvPortMalloc(size_t size, const char* file, int line)
357357
{
358358
HeapSelectDram ephemeral;
359-
return heap_pvPortMalloc(size, file, line);;
359+
return heap_pvPortMalloc(size, file, line);;
360360
}
361361

362362
void* IRAM_ATTR pvPortCalloc(size_t count, size_t size, const char* file, int line)
363363
{
364364
HeapSelectDram ephemeral;
365-
return heap_pvPortCalloc(count, size, file, line);
365+
return heap_pvPortCalloc(count, size, file, line);
366366
}
367367

368368
void* IRAM_ATTR pvPortRealloc(void *ptr, size_t size, const char* file, int line)
369369
{
370370
HeapSelectDram ephemeral;
371-
return heap_pvPortRealloc(ptr, size, file, line);
371+
return heap_pvPortRealloc(ptr, size, file, line);
372372
}
373373

374374
void* IRAM_ATTR pvPortZalloc(size_t size, const char* file, int line)
375375
{
376376
HeapSelectDram ephemeral;
377-
return heap_pvPortZalloc(size, file, line);
377+
return heap_pvPortZalloc(size, file, line);
378378
}
379379

380380
void IRAM_ATTR vPortFree(void *ptr, const char* file, int line)
@@ -384,7 +384,62 @@ void IRAM_ATTR vPortFree(void *ptr, const char* file, int line)
384384
// correct context. umm_malloc free internally determines the correct heap.
385385
HeapSelectDram ephemeral;
386386
#endif
387-
return heap_vPortFree(ptr, file, line);
387+
return heap_vPortFree(ptr, file, line);
388388
}
389389

390+
#if (NONOSDK >= (0x30000))
391+
////////////////////////////////////////////////////////////////////////////////
392+
/*
393+
New for NON-OS SDK 3.0.0 and up
394+
Needed for WPA2 Enterprise support. This was not present in SDK pre 3.0
395+
396+
The NON-OS SDK 3.0.x has breaking changes to pvPortMalloc. They added one more
397+
argument for selecting a heap. To avoid breaking the build, I renamed their
398+
broken version pvEsprMalloc. To be used, the LIBS need to be edited.
399+
400+
They also added pvPortZallocIram and pvPortCallocIram, which are not a
401+
problem.
402+
403+
WPA2 Enterprise connect crashing is fixed at v3.0.2 and up.
404+
405+
Not used for unreleased version NONOSDK3V0.
406+
*/
407+
void* IRAM_ATTR sdk3_pvPortMalloc(size_t size, const char* file, int line, bool iram)
408+
{
409+
if (iram) {
410+
HeapSelectIram ephemeral;
411+
return heap_pvPortMalloc(size, file, line);
412+
} else {
413+
HeapSelectDram ephemeral;
414+
return heap_pvPortMalloc(size, file, line);
415+
}
416+
}
417+
418+
void* IRAM_ATTR pvPortCallocIram(size_t count, size_t size, const char* file, int line)
419+
{
420+
HeapSelectIram ephemeral;
421+
return heap_pvPortCalloc(count, size, file, line);
422+
}
423+
424+
void* IRAM_ATTR pvPortZallocIram(size_t size, const char* file, int line)
425+
{
426+
HeapSelectIram ephemeral;
427+
return heap_pvPortZalloc(size, file, line);
428+
}
429+
430+
/*
431+
uint32_t IRAM_ATTR user_iram_memory_is_enabled(void)
432+
{
433+
return CONFIG_ENABLE_IRAM_MEMORY;
434+
}
435+
436+
We do not need the function user_iram_memory_is_enabled().
437+
1. It was used by mem_manager.o which was replaced with this custom heap
438+
implementation. IRAM memory selection is handled differently.
439+
2. In libmain.a, Cache_Read_Enable_New uses it for cache size. However, When
440+
using IRAM for memory or running with 48K IRAM for code, we use a
441+
replacement Cache_Read_Enable to correct the cache size ignoring
442+
Cache_Read_Enable_New's selected value.
443+
*/
444+
#endif
390445
};

cores/esp8266/wpa2_eap_patch.cpp

+55-6
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,44 @@
55
* modules.
66
*
77
*/
8-
98
#include <string.h>
109
#include <ets_sys.h>
1110
#include <pgmspace.h>
1211
#include "coredecls.h"
1312

13+
#if defined(NONOSDK22x_190703) || \
14+
defined(NONOSDK22x_191122) || \
15+
defined(NONOSDK22x_191105) || \
16+
defined(NONOSDK22x_191124) || \
17+
defined(NONOSDK22x_190313) || \
18+
defined(NONOSDK221) || \
19+
defined(NONOSDK3V0) || \
20+
defined(NONOSDK300) || \
21+
defined(NONOSDK301) || \
22+
defined(NONOSDK302) || \
23+
defined(NONOSDK303) || \
24+
defined(NONOSDK304) || \
25+
defined(NONOSDK305)
26+
27+
// eap_peer_config_deinit() - For this list of SDKs there are no significant
28+
// changes in the function. Just the line number reference for when vPortFree
29+
// is called. When vPortFree is called, register a12 continues to hold a pointer
30+
// to the struct StateMachine. Our cleanup routine should continue to work.
31+
#if defined(NONOSDK300) || defined(NONOSDK301)
32+
// Minor changes only line number changed
33+
#define SDK_LEAK_LINE 809
34+
#elif defined(NONOSDK302) || defined(NONOSDK303) || defined(NONOSDK304)
35+
// Minor changes only line number changed
36+
#define SDK_LEAK_LINE 831
37+
#elif defined(NONOSDK305)
38+
// At v3.0.5 Espressif moved `.text.eap_peer_config_deinit` to
39+
// `eap_peer_config_deinit` then later in latest git they moved it
40+
// back. For our linker script both are placed in flash.
41+
#define SDK_LEAK_LINE 831
42+
#else
43+
#define SDK_LEAK_LINE 799
44+
#endif
45+
1446
#ifdef DEBUG_WPA2_EAP_PATCH
1547
#include "esp8266_undocumented.h"
1648
#define DEBUG_PRINTF ets_uart_printf
@@ -100,7 +132,7 @@ struct StateMachine { // size 200 bytes
100132
* same line.
101133
*/
102134
void patch_wpa2_eap_vPortFree_a12(void *ptr, const char* file, int line, void* a12) {
103-
if (799 == line) {
135+
if (SDK_LEAK_LINE == line) {
104136
// This caller is eap_peer_config_deinit()
105137
struct StateMachine* sm = (struct StateMachine*)a12;
106138
if (ptr == sm->config[0]) {
@@ -126,21 +158,38 @@ void patch_wpa2_eap_vPortFree_a12(void *ptr, const char* file, int line, void* a
126158
}
127159
#endif
128160
}
129-
#if 0
130-
// This is not needed because the call was NO-OPed in the library. This code
131-
// snippit is just to show how a future memory free issue might be resolved.
132-
else if (672 == line) {
161+
162+
#if defined(NONOSDK300) || defined(NONOSDK301)
163+
else if (682 == line) {
133164
// This caller is wpa2_sm_rx_eapol()
134165
// 1st of a double free
135166
// let the 2nd free handle it.
136167
return;
137168
}
169+
#elif defined(NONOSDK302) || defined(NONOSDK303) || defined(NONOSDK304) || defined(NONOSDK305)
170+
// It looks like double free is fixed. WPA2 Enterpise connections work
171+
// without crashing. wpa2_sm_rx_eapol() has a few changes between NONOSDK301
172+
// and NONOSDK302. However, this set of releases still have memory leaks.
173+
#else
174+
// This is not needed because the call was NO-OPed in the library.
175+
// Keep code snippit for reference.
176+
// else if (672 == line) {
177+
// // This caller is wpa2_sm_rx_eapol()
178+
// // 1st of a double free
179+
// // let the 2nd free handle it.
180+
// return;
181+
// }
138182
#endif
139183
vPortFree(ptr, file, line);
140184
}
141185

142186
};
143187

188+
#else
189+
#error "Internal error: A new SDK has been added. This module must be updated."
190+
#error " Need to test WPA2 Enterpise connectivity."
191+
#endif
192+
144193
/*
145194
* This will minimize code space for non-wifi enterprise sketches which do not
146195
* need the patch and disable_extra4k_at_link_time().

0 commit comments

Comments
 (0)