Skip to content

storage: fix valid partion check with windows formatted sd card #8346

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
Merged
merged 1 commit into from
Oct 26, 2018
Merged

storage: fix valid partion check with windows formatted sd card #8346

merged 1 commit into from
Oct 26, 2018

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Oct 9, 2018

Description

For Windows-formatted SD card, it is not partitioned (no MBR), but its PBR has the same boot signature (0xaa55) as MBR. MBRBlockDevice would easily mis-recognize this SD card has valid partitions if only partition type is checked. This PR adds check by only accepting 0x00 (inactive)/0x80 (active) for valid partition status.

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@ccli8
Fix mis-recognize that Windows-formatted SD card has valid partitions
8ef23ff 
For Windows-formatted SD card, it is not partitioned (no MBR), but its PBR has the
same boot signature (0xaa55) as MBR. We would easily mis-recognize this SD card has valid
partitions if we only check partition type. We add check by only accepting 0x00 (inactive)
/0x80 (active) for valid partition status.
@0xc0170 0xc0170 requested a review from a team October 9, 2018 14:10
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 16, 2018

@ARMmbed/mbed-os-storage Please review

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 24, 2018

@ARMmbed/mbed-os-storage Please review

Reminder to review this one

@dannybenor
Copy link

@geky can you help with this one?

@0xc0170 0xc0170 requested a review from geky October 24, 2018 13:21
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 24, 2018

@ccli8 Can you rephrase : Fix mis-recognize that Windows-formatted SD card has valid partitions please ?

@geky
Copy link
Contributor

geky commented Oct 24, 2018

This is a good idea 👍 Thanks for submitting a PR.

@ccli8
Copy link
Contributor Author

ccli8 commented Oct 25, 2018

Can you rephrase : Fix mis-recognize that Windows-formatted SD card has valid partitions

@0xc0170 Rephrase what?

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 25, 2018

The titles are used in the release notes. I would rephrase to similar storage: fix win format sd valid partion check

@cmonr
Copy link
Contributor

cmonr commented Oct 25, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 25, 2018

Build : SUCCESS

Build number : 3466
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8346/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@ccli8 ccli8 changed the title Fix mis-recognize that Windows-formatted SD card has valid partitions storage: fix valid partion check with windows formatted sd card Oct 26, 2018
@ccli8
Copy link
Contributor Author

ccli8 commented Oct 26, 2018

@0xc0170 I change the title to storage: fix valid partion check with windows formatted sd card.

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2018

Exporter Build : SUCCESS

Build number : 3089
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8346/

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2018

Test : SUCCESS

Build number : 3257
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8346/3257

@0xc0170 0xc0170 merged commit fc741f0 into ARMmbed:master Oct 26, 2018
@ccli8 ccli8 deleted the nuvoton_fix_mbr branch October 26, 2018 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr approved these changes

@geky geky geky approved these changes

Assignees
No one assigned
Labels
release-version: 5.10.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ccli8 @0xc0170 @dannybenor @geky @cmonr @mbed-ci