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

Do not check and lock bootloader sector write protection on every boot [ch17416] #1578

Merged
merged 1 commit into from Sep 28, 2018

Conversation

@technobly
Copy link
Member

commented Sep 28, 2018

Problem

Checking and locking the bootloader sector write protection (if needed) on every boot could be contributing to flash memory corruption, although no testing has shown this to occur. It has only occurred naturally in the wild. See references.

Solution

Do not check and lock bootloader write protect on every boot

The idea being that a power glitch could result in the read of sector 0 write protection bits being misinterpreted as unprotected, which would unlock the Option Bytes register to change these bits to be protected. While writing to Option Bytes register, if power is lost or the MCU is reset, this can result in Read Protection level being set to 1. This is fairly well understood and easy to reproduce, so not attempting to write protect the bootloader on every boot is a great mitigation technique to avoiding RPD level 1.

We still do write protect the bootloader at the time of MFG. and also on future bootloader updates. Read Protection level 1 is not really harmful though and the device can function normally (minus debugging tool support in this mode). The real issue is if Read Protection level 1 is subsequently set back to level 0, which causes the MCU to self-mass-erase. This is a fairly difficult thing to do since RDP level 0 requires 0xAA to be written to the Option Bytes register, and is not something done in system firmware anywhere.

Steps to Test

  • Compile a test application with debugging enabled to make using a debugger easier
  • firmware/modules $ make clean all -s PLATFORM=electron COMPILE_LTO=n USE_SWD_JTAG=y DEBUG_BUILD=y APP=tinker program-dfu
  • run the unprotect command below
  • run the flashinfo command below and verify that the first sector is unprotected
  • Reset the device
  • run the flashinfo command below and verify that the first sector is still unprotected
  • Flash a bootloader via OTA or YModem
  • run the flashinfo command below and verify that the first sector is now protected
  • Reset the device
  • run the flashinfo command below and verify that the first sector is still protected

Some bash functions to make working with OpenOCD easier

unprotect() {
  openocd -f interface/jlink.cfg \
        -c "transport select swd" \
        -f "target/stm32f2x.cfg" \
	-c "init" \
	-c "reset init" \
	-c "stm32f2x unlock 0" \
	-c "flash protect 0 0 last off" \
	-c "reset run" \
	-c "shutdown"
}

flashinfo() {
  openocd -f interface/jlink.cfg \
        -c "transport select swd" \
        -f "target/stm32f2x.cfg" \
        -c "init" \
        -c "reset init" \
        -c "flash info 0" \
        -c "reset" \
        -c "shutdown"
}

References

https://community.particle.io/t/bug-bounty-electron-not-booting-after-battery-discharges-completely/25043


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • [N/A] Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)
do not check and lock bootloader write protect on every boot
The idea being that a power glitch could result in the read of sector 0 write protection bits being misinterpreted as unprotected, which would unlock the Option Bytes register to change these bits to be protected.  While writing to Option Bytes register, if power is lost or the MCU is reset can result in Read Protection level being set to 1. This is fairly well understood and easy to reproduce, so not attempting to write protect the bootloader on every boot is a great mitigation techinque to avoiding RPD level 1.  We still do write protect the bootloader at the time of MFG. and also on future bootloader updates.  Read Protection level 1 is not really harmful though and the device can function normally (minus debugging tool support in this mode).  The real issue is if Read Protection level 1 is subsequently set back to level 0, which causes the MCU to self-mass-erase.  This is a fairly difficult thing to do since RDP level 0 requires 0xAA to be written to the Option Bytes register, and is not something done in system firmware anywhere.

@technobly technobly added this to the 0.8.0-rc.11 milestone Sep 28, 2018

@technobly technobly requested a review from m-mcgowan Sep 28, 2018

@technobly technobly merged commit 44abc71 into develop Sep 28, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@technobly technobly deleted the fix/bootloader-lock-on-boot branch Sep 28, 2018

@kubark42

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2018

Nice find. From experience, I imagine that that single line of code took oodles of hard work to track down.

This bug is three years old, 8e02f00. Will the fix be backported to other firmware releases as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.