Skip to content

Fix RAM corruption caused by our hook of register_chipv6_phy(init_data*) #1277

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

Merged
merged 1 commit into from
Dec 22, 2015

Conversation

alltheblinkythings
Copy link
Contributor

"init_data", when non-NULL, 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.

Move phy_init_data to flash as it's now readonly and never modified.

…a*).

"init_data", when non-NULL, 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.

Move phy_init_data to flash as it's now readonly and never modified.
@alltheblinkythings
Copy link
Contributor Author

Retry of #1276 . The issue was a null ptr dereference as only the first call to register_chipv6_phy passes a non-null init_data (and more are made after some runtime).

igrr added a commit that referenced this pull request Dec 22, 2015
Fix RAM corruption caused by our hook of register_chipv6_phy(init_data*)
@igrr igrr merged commit 8caf70c into esp8266:master Dec 22, 2015
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.

3 participants