espressif / ESP8266_NONOS_SDK

ESP8266 nonOS SDK
Other
927 stars 536 forks source link

WPA Enterprise tries to free invalid memory #350

Open Flole998 opened 3 years ago

Flole998 commented 3 years ago

For some reason there is a function which attempts to free an invalid range of memory. This leads to the following coredump:

No poison after block at: 0x406e4a2f, actual data: 0x0 0x0 0x80 0x0

User exception (panic/abort/assert)
--------------- CUT HERE FOR EXCEPTION DECODER ---------------

 Error
   :?:::0x4024b080:etharp_output
   :?:::0x40201a52:raise_exception
   :?:::0x40201aaf:__panic_func
   0x40100e54 get_unpoisoned_check_neighbors
   0x401012f2 umm_free
   :\packages\esp8266\hardware\esp8266\3.0.0\cores\esp8266\umm_malloc/umm_malloc.cpp:574
   0x4010130d umm_poison_free_fl
   :?:::0x4024c4e0:etharp_output
   :?:::0x40229979:wpa_set_bss
   0x40100827 HeapSelectDram
   :\packages\esp8266\hardware\esp8266\3.0.0\cores\esp8266/heap.cpp:370
   :?:::0x4022aac7:wpabuf_free
   :?:::0x4022bb92:wpa2_sm_rx_eapol
   :?:::0x4022bba6:wpa2_sm_rx_eapol
   :?:::0x4022bbe1:wpa2_sm_rx_eapol
   :?:::0x4022b7ac:wpa2_sm_rx_eapol
   :?:::0x40223129:sta_input
   :?:::0x40240ccf:pp_tx_idle_timeout
   :?:::0x4024058f:ppPeocessRxPktHdr
   : ?? ??:0
   : ?? ??:0
   :?:::0x40105b88:call_user_start_local
   :?:::0x40105b8e:call_user_start_local
   :?:::0x4010000d:call_user_start
   0x40235458 cont_ret
   0x40235411 cont_continue

This issue exists since SDK > 2.3.0, so i am stuck on that version for now.

The code being used is based on the Arduino implementation (and this issue is tracked there as https://github.com/esp8266/Arduino/issues/8082)

void setup() {

    Serial.begin(115200);
    Serial.println("Startup!");

    Serial.print("Heap Free: ");
    Serial.println(system_get_free_heap_size());

    enableWiFiAtBootTime();

    wifi_set_opmode_current(STATION_MODE);

    struct station_config wifi_config;

    memset(&wifi_config, 0, sizeof(wifi_config));
    strcpy((char*)wifi_config.ssid, ssid);
    wifi_station_set_config_current(&wifi_config);

    wifi_station_set_wpa2_enterprise_auth(1);

    wifi_station_set_enterprise_identity((uint8_t*)identity, strlen(identity));
    wifi_station_set_enterprise_cert_key(esp_cert_pem, esp_cert_pem_len, esp_key_pem, esp_key_pem_len, NULL, 1);

    wifi_station_disconnect();
    wifi_station_connect();

    Serial.println(F("Waiting for connection..."));

    while (WiFi.status() != WL_CONNECTED) {
        if (millis() > 60000) {
            Serial.println(F("Took wayyy to long. Restarting..."));
            ESP.restart();
        }

        delay(1000);
    }
}

void loop() {
         Serial.println("Connected!");
}
Flole998 commented 3 years ago

@xcguang Any chance to get this fixed? This breaks WPA Enterprise completely in newer SDK Versions. Please have a look at what's going on there and why invalid memory is freed.

vanThijs commented 3 years ago

I am experiencing the same issue, WPA2 enterprise connections fail most of the time (roughly 1 out of 10 tries succeeds in connecting).

eriksl commented 2 years ago

Is this really an SDK issue, or actually a bug in Arduino...

Flole998 commented 2 years ago

@eriksl The SDK is attempting to call free on an invalid memory address. That is not a bug in Arduino.

Flole998 commented 2 years ago

@xcguang Could you please have a look at this? This breaks WPA Enterprise completely in recent versions, this should really be addressed and it's probably an easy fix.

eriksl commented 2 years ago

@Flole998 how are you sure?

Flole998 commented 2 years ago

@eriksl Because it's the SDK that's calling free() on an invalid address. See how the stacktrace goes basically only through SDK functions and nothing Arduino related? There's simply an attempt to free invalid memory in wpabuf_free() which someone should finally fix, then everything would be fine (as someone else who had this issue simply removed the abort() when freeing invalid memory, but seriously, is that how a good and clean SDK looks like?).

Flole998 commented 2 years ago

@ustccw Could you please have a look into this? It's probably a super easy fix for you to figure out why free() is called on an invalid memory address here by the SDK.

eriksl commented 2 years ago

@Flole998 that could be plausible. There are more horrible bugs in de SDK code.

Apparently I never hit it because I am not using WPA-Enterprise.

Flole998 commented 2 years ago

I've found the bug and patched it in the binary in the meantime. There are other bugs aswell in the code that I spotted when reading the assembly (several memory leaks for example), so in general code quality seems pretty low and now I understand why they prefer to keep it closed source aswell.

eriksl commented 2 years ago

That is exactly my experience, the quality of the code, both open as closed is not terribly good. I guess the Espressif guys are great at designing hardware, not so good in writing software.

I would really love it if Espressif would take the following actions (and there is no reason why they could not):

Then we could make a community-based SDK where a LOT of bugs could be solved and, I am sure, a lot of code could be improved / made more efficient.

In fact, for my stuff, I don't really need the SDK code. It's just that I don't have the proper information to do all the initialisation in my own code, which I'd prefer. I already have my own lwip-fork.

Speaking about which, I'd be great if Espressif would finally disclose the interface between SDK/hardware and LWIP, so would take a shot at adapting a recent LWIP version. The "current" version is something like eight years old...