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.

Sign up for GitHub

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

Jump to bottom

Fixed PUYA flash write buffer alignment #7491

Merged
merged 3 commits into from
Jul 30, 2020
Merged

Fixed PUYA flash write buffer alignment #7491

merged 3 commits into from
Jul 30, 2020

Conversation

drzony
Copy link
Contributor

@drzony drzony commented Jul 29, 2020 •
edited
Loading

This merge fixes flash writing on PUYA flashes when write size is not a multiple of 4.
This bug prevents updating via Arduino OTA when using gzipped image.

earlephilhower
earlephilhower approved these changes Jul 30, 2020
View reviewed changes
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! The rounding up to x4 is a little odd but I think it works fine.

cores/esp8266/Esp.cpp Outdated Show resolved Hide resolved
@devyte devyte merged commit 85ea47e into esp8266:master Jul 30, 2020
@devyte devyte added this to the 3.0.0 milestone Jul 30, 2020
d-a-v added a commit that referenced this pull request Aug 2, 2020
@d-a-v
backport #7491
2843a5a
@yumkam
Copy link

yumkam commented Aug 7, 2020 •
edited
Loading

FTR, this over-reads data by up to 3 bytes. Assuming data is 4-bytes aligned and does not point to some kind of "special" memory (mmio/etc), this should be practically harmless. But formally this is still undefined behavior and compiler is free to kill kittens and start WWIII.
P.S. and after a bit more looking at the code, this looks incorrect to write junk past size to flash.

@drzony drzony mentioned this pull request Aug 7, 2020
Merged
@drzony
Copy link
Contributor Author

drzony commented Aug 7, 2020

Thanks for the observation @yumkam , must have missed this somehow. Another pull request is on the way.

@drzony
Copy link
Contributor Author

drzony commented Aug 7, 2020 •
edited
Loading

As for the overreading I have not seen a flash that is not 4-byte aligned. All the functions for spi_flash ops have uint32 * as parameters, but it's fixed anyway in pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@earlephilhower earlephilhower earlephilhower approved these changes

@d-a-v d-a-v d-a-v approved these changes

@devyte devyte devyte approved these changes

Assignees
No one assigned
Labels
component: core type: bug
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@drzony @yumkam @earlephilhower @d-a-v @devyte