Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORRUPT HEAP when send large string over BLE protobuf (IDFGH-1350) #3633

Closed
1 of 3 tasks
oluckyman opened this issue Jun 15, 2019 · 6 comments
Closed
1 of 3 tasks

CORRUPT HEAP when send large string over BLE protobuf (IDFGH-1350) #3633

oluckyman opened this issue Jun 15, 2019 · 6 comments

Comments

@oluckyman
Copy link

oluckyman commented Jun 15, 2019

Environment

  • Development Kit: [none]
  • Module or chip used: [ESP32-WROOM-32]
  • IDF version (run git describe --tags to find it):
    // v3.3-beta3
    // also tried at v3.2
  • Build System: [Make]
  • Compiler version (run xtensa-esp32-elf-gcc --version to find it):
    // 5.2.0
  • Operating System: [macOS]
  • Power Supply: [USB]

Problem Description

I mixed two examples: ble_prov and custom_config
I send wifi config from my iOS app and ble_prov handler receives it.
Then I send custom_config request from my iOS app and custom_config handler receives it.
custom_config has example request:

message CustomConfigRequest {
    string info = 1;
    int32 version = 2;
}

When I send request with info string long enough (253 bytes to be exact) the program crashes with CORRUPT HEAP error.

Expected Behavior

It should not crush. It should show in the monitor "Custom config received" the same as it does for the shorter strings:

I (1143) app_prov: BLE Provisioning started
I (886393) app_prov_handler: WiFi Credentials Received : 
        ssid WIFI_8728 
        password 77c47fxymwye
I (886573) app_prov_handler: Custom config received :
        Info : A string which is shorter than 253 bytes
        Version : 42

Actual Behavior

It crushes with CORRUPT HEAP error:

I (64913) app_prov_handler: WiFi Credentials Received : 
        ssid WIFI_8728 
        password 77c47fxymwye
CORRUPT HEAP: Bad tail at 0x3ffbb44d. Expected 0xbaad5678 got 0xbaad562a
assertion "head != NULL" failed: file "/Users/oluckyman/Projects/esp/esp-idf/components/heap/multi_heap_poisoning.c", line 214, function: multi_heap_free
abort() was called at PC 0x401084cf on core 0
0x401084cf: __assert_func at /Users/ivan/e/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdlib/../../../.././newlib/libc/stdlib/assert.c:63 (discriminator 8)

Steps to repropduce

I'll attach code, but it's just the code from examples mixed together. There is no my code at all. Also I inserted heap integrity checks in every function and it's fine everywhere.

    if (!heap_caps_check_integrity_all(true)) {
        ESP_LOGE("MEM CHECK", "At least one heap is corrupt");
    } else {
        ESP_LOGE("MEM CHECK", "Heap is fine!");
    }

The requests are sent from my iOS app, but I think you can use python script to send requests.
First it sends wifi ssid and passphrase as in ble_prov example. Then it sends custom config request with info and version as in custom_config example.

UPDATE

Just tried to send 253 chars long string as wifi SSID. And it crushes the same way. So I think you can just take ble_prov example as is and send a long string as wifi credentials.

Code to reproduce this issue

https://github.com/oluckyman/esp-idf-ble_prov_and_custom_config

Debug Logs

I (64913) app_prov_handler: WiFi Credentials Received : 
        ssid WIFI_8728 
        password 77c47fxymwye
CORRUPT HEAP: Bad tail at 0x3ffbb44d. Expected 0xbaad5678 got 0xbaad562a
assertion "head != NULL" failed: file "/Users/oluckyman/Projects/esp/esp-idf/components/heap/multi_heap_poisoning.c", line 214, function: multi_heap_free
abort() was called at PC 0x401084cf on core 0
0x401084cf: __assert_func at /Users/ivan/e/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdlib/../../../.././newlib/libc/stdlib/assert.c:63 (discriminator 8)


ELF file SHA256: fd84649b09c49ccf4672cace39575b0fba4958bee7ec8c52ef48830174534346

Backtrace: 0x4008d658:0x3ffd8080 0x4008d8a5:0x3ffd80a0 0x401084cf:0x3ffd80c0 0x40096a33:0x3ffd80f0 0x40085b4e:0x3ffd8110 0x40085f25:0x3ffd8130 0x40085fd9:0x3ffd8160 0x4011c3ce:0x3ffd8180 0x4011c4f1:0x3ffd8400 0x4011ca5b:0x3ffd8440 0x401269fe:0x3ffd8480 0x401226f2:0x3ffd84c0 0x40093501:0x3ffd84f0
0x4008d658: invoke_abort at /Users/oluckyman/Projects/esp/esp-idf/components/esp32/panic.c:715

0x4008d8a5: abort at /Users/oluckyman/Projects/esp/esp-idf/components/esp32/panic.c:715

0x401084cf: __assert_func at /Users/ivan/e/newlib_xtensa-2.2.0-bin/newlib_xtensa-2.2.0/xtensa-esp32-elf/newlib/libc/stdlib/../../../.././newlib/libc/stdlib/assert.c:63 (discriminator 8)

0x40096a33: multi_heap_free at /Users/oluckyman/Projects/esp/esp-idf/components/heap/multi_heap_poisoning.c:344

0x40085b4e: heap_caps_free at /Users/oluckyman/Projects/esp/esp-idf/components/heap/heap_caps.c:404

0x40085f25: trace_free at /Users/oluckyman/Projects/esp/esp-idf/components/heap/heap_trace.c:188

0x40085fd9: __wrap_free at /Users/oluckyman/Projects/esp/esp-idf/components/heap/heap_trace.c:401

0x4011c3ce: prepare_write_event_env at /Users/oluckyman/Projects/esp/esp-idf/components/protocomm/src/transports/protocomm_ble.c:174

0x4011c4f1: transport_simple_ble_write at /Users/oluckyman/Projects/esp/esp-idf/components/protocomm/src/transports/protocomm_ble.c:193

0x4011ca5b: gatts_profile_event_handler at /Users/oluckyman/Projects/esp/esp-idf/components/protocomm/src/simple_ble/simple_ble.c:100

0x401269fe: btc_gatts_cb_to_app at /Users/oluckyman/Projects/esp/esp-idf/components/bt/bluedroid/btc/profile/std/gatt/btc_gatts.c:54
 (inlined by) btc_gatts_cb_handler at /Users/oluckyman/Projects/esp/esp-idf/components/bt/bluedroid/btc/profile/std/gatt/btc_gatts.c:802

0x401226f2: btc_task at /Users/oluckyman/Projects/esp/esp-idf/components/bt/bluedroid/btc/core/btc_task.c:110

0x40093501: vPortTaskWrapper at /Users/oluckyman/Projects/esp/esp-idf/components/freertos/port.c:403


Rebooting...

Other items if possible

  • sdkconfig file (attach the sdkconfig file from your project folder) // it's in the repo
  • elf file in the build folder (note this may contain all the code details and symbols of your project.) // will upload on demand
  • coredump (This provides stacks of tasks.) // not sure what it is
@github-actions github-actions bot changed the title CORRUPT HEAP when send large string over BLE protobuf CORRUPT HEAP when send large string over BLE protobuf (IDFGH-1350) Jun 15, 2019
@oluckyman
Copy link
Author

Just tried to send 253 chars long string as wifi SSID. And it crushes the same way. So I think you can just take ble_prov example as is and send a long string as wifi credentials to reproduce that crash

@anurag-kar
Copy link
Contributor

@oluckyman Thanks for reporting the issue. There are two bugs involved here:

  1. No upper limit on WiFi SSID /Passphrase length (Fix for this is already merged in the internal repo and should reflect on github soon)
  2. Unbound memcpy into protocomm_ble prepare write buffer

Here's a patch combining both bugfixes. Please let me know if that fixes the problem. If so, I'll raise an internal merge request for the second bugfix.

@oluckyman
Copy link
Author

Thanks for the quick response!
Just tried the patch. Now it does not corrupt the heap. It shows in the monitor this error:

E (28492) protocomm_ble: Error appending to prepare buffer

It shows this error both for long Wi-Fi SSID and for long custom info field.
Nothing happens after this error.

@anurag-kar
Copy link
Contributor

anurag-kar commented Jun 19, 2019

Yes. That is because protocomm_ble is limited by 256 byte of transaction limit. This is imposed by the maximum BLE attribute length hardcoded into protocomm_ble, so no way to change that. In any case BLE attribute length is limited to 512 as per standard. In case you want to achieve longer transaction size, have the protocol send / receive data in pieces.

Edit:

It shows this error both for long Wi-Fi SSID and for long custom info field.

Wi-Fi SSID and Passphrase are limited to 32 and 63 bytes (as per standard), respectively, so cannot make these any longer. There is no limit on the custom info field, so that can be as long as possible, given the packet size is <= 256 bytes.

@anurag-kar
Copy link
Contributor

There is an internal merge request under review that should allow for longer than 512 byte transactions, but it will take some time to finalize.

@bbinet
Copy link
Contributor

bbinet commented Jan 29, 2021

@anurag-kar I would also like to send/receive requests/responses which size is bigger than 512 bytes, so I'm interested in your last comment: #3633 (comment)

What is the status for the internal merge request that should allow for longer than 512 byte transactions and which you were talking about ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants