Skip to content

Conversation

alltheblinkythings
Copy link
Contributor

The "init_data" pointer is on the heap, and the register_chipv6_phy call
sometimes modifies data in (at least) the offset range [128:249], suggesting
that it is a buffer larger than 128 bytes in size (the size of our
"phy_init_data" buffer). When we use our static buffer (prior to this
change), the call could would overwrite the .rodata section and lead to
undefined behaviour.

To address this, just patch the heap-allocated buffer with our data.

…a*).

The "init_data" pointer is on the heap, and the register_chipv6_phy call
sometimes modifies data in (at least) the offset range [128:249], suggesting
that it is a buffer larger than 128 bytes in size (the size of our
"phy_init_data" buffer).  When we use our static buffer (prior to this
change), the call could would overwrite the .rodata section and lead to
undefined behaviour.

To address this, just patch the heap-allocated buffer with our data.
@alltheblinkythings
Copy link
Contributor Author

Trivially, on ESP-202 (at least), without this change the code just below results in an empty string for system_get_sdk_version() and the device cannot connect to WiFi afterwards.

extern "C" {
#include "user_interface.h"
}

void setup() {
  Serial.begin(115200);
  delay(5000);
  Serial.printf("\nsystem_get_sdk_version(): %s\n", system_get_sdk_version());
}

void loop() {
  delay(30000);
}

The cause is the SDK_VERSION[] in rodata being overwritten during startup. Adding appropriately placed guards in RAM, I determined the corruption to occur cause to be around this call:

static uint8_t phy_init_data[128] = { ... }; 
extern int __wrap_register_chipv6_phy(uint8_t* unused /* init_data */) {
    phy_init_data[107] = __get_adc_mode();
    return __real_register_chipv6_phy(phy_init_data);
}

The call to __real_register_chipv6_phy() modifies data at phy_init_data[128:241], which overlaps with .rodata.

Investigating further, I found that the init_data being passed in there is actually a heap allocated buffer (which defaults to some data present in libmain.a), so it seems like we should just patch that directly, as done here.

I'm curious, though: given that we're being passed in a buffer that's initialized by libmain.a with defaults, why do we hook this? Is it to make us ignore the values in flash? (The SDK does appear to write the data from libmain.a back to flash if it happens to be blank.)

@alltheblinkythings
Copy link
Contributor Author

Cancel this: it seems that this causes heap corruption later on. I'm not sure what is going on here and why this device stops working with 1.5.0 SDK - will open an issue to track and close this.

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

Successfully merging this pull request may close these issues.

1 participant