adafruit / tinyuf2

UF2 bootloader based on TinyUSB for embedded devices such as ESP32S2, STM32F4 and iMX RT10xx
MIT License
326 stars 178 forks source link

[STM32F4] Add bootloader overwrite self-protection #189

Closed KarlK90 closed 2 years ago

KarlK90 commented 2 years ago

For review read the commits in separation

Driven by a remark that bootloader protection is still pending I implemented it for the STM32F4 family.

For broader use a new board level function is introduced board_flash_protect_bootloader(). All other ports are currently implemented as stubs.

On the STM32F4 family tinyuf2 occupies the first two flash sectors. The overwrite protection is enabled by setting the option bytes nWRP register in the flash peripheral. This is done when booting the board to always prevent tinyuf2 being overwritten. Also this is also only done for the bits that haven't been already processed to not wear out the memory.

@hathach I'm not sure if this logic should be included for all ports and moved to tinyuf2's main.c?

By default the overwrite protection is disabled by setting PROTECT_BOOTLOADER to 0.

Disabling the write protection again i.e. changing the nWRP to 1 again, can be done with a simple program called tinyuf2-unlocker that can be uploaded as an UF2 image or has do be done externally via DFU or SWD with STM32CubeProg or openocd.

KarlK90 commented 2 years ago

Ready for review.

KarlK90 commented 2 years ago

I have written a small program that is apply-able as a UF2 image that will unprotected the flash sectors again and jump to the built-in DFU bootloader of the STM32F411. This acts as unlocker key to update or overwrite the whole flash.

The project is called tinyuf2-unlocker

hathach commented 2 years ago

Hi thank you for your PR. I am currently on TET (Lunar New Year) holiday. Will check this out later on

KarlK90 commented 2 years ago

Thanks, have a great holiday!

sigprof commented 2 years ago

Hmm, is reserving the whole 64KB block for the bootloader intended? Some of these 16KB flash sectors could be useful for storing some firmware settings (unfortunately, on STM32F4 other flash sectors are much larger, therefore rewriting them at runtime is problematic). One of recent QMK PRs actually tried to add a board which uses tinyuf2, but assumes that only sectors 0 and 1 are occupied by the bootloader, and therefore uses sector 2 for EEPROM emulation. Setting the protection flags for sectors 0…3 irrespective of the actual bootloader size would break such usage.

ladyada commented 2 years ago

for eeprom emulation they should use the last block in memory

megamind4089 commented 2 years ago

@KarlK90 Thanks for this PR. I thought, the linker uses only 32 KB of flash which is first 2 sectors and use the next 2 sectors for EEPROM emu.

https://github.com/adafruit/tinyuf2/blob/master/ports/stm32f4/linker/stm32f4.ld#L47

Am i missing something ?

KarlK90 commented 2 years ago

@KarlK90 Thanks for this PR. I thought, the linker uses only 32 KB of flash which is first 2 sectors and use the next 2 sectors for EEPROM emu.

https://github.com/adafruit/tinyuf2/blob/master/ports/stm32f4/linker/stm32f4.ld#L47

Am i missing something ?

No you are completely right, I took the number from the QMK linker file which starts the usable flash sections at 64kb offset. I didn't cross reference it with the tinyuf2 linker script. Fixed the bootloader protection by only locking the first two sectors.

KarlK90 commented 2 years ago

Although the tinyuf2 bootloader assumes that the main app starts at a 64kb offset, so technically there is a 32kb gap...

https://github.com/adafruit/tinyuf2/blob/master/ports/stm32f4/boards.h#L38

KarlK90 commented 2 years ago

Friendly ping @hathach

sigprof commented 2 years ago

If you are considering to make board_flash_protect_bootloader(false) a generic API, note that it is actually impossible to implement for some MCUs. In fact, STM32F4xx almost looks like an outlier, because for many other STM32 chips (F103, F303, L4xx, L072, probably some more) the only way to make new option bytes take effect is to perform a system reset (or an “option byte reload” procedure, which itself does a system reset); at least that's what the official documentation says. So the implementation of board_self_update() suggested above won't work for those MCUs.

Implementing the self-update procedure on MCUs like that would still be possible, but would require that the bootloader does not reapply the flash protection when the firmware reboot is requested (this seems to be the case with the current code, because board_flash_init() is not called if check_dfu_mode() returns false). There would also be some potential for infinite loop if the disabled protection status does not stick for any reason.

hathach commented 2 years ago

@sigprof thank for your input, do you think we can use one of the DBL_TAP_MAGIC_PROTECTION_OFF as way to go around the infinite loop and continue to with the update once protection if off. It is definitely need some handshake between self-update app and bootloader.

However, I think we should stick with simplest way that work with F4 first, and generalize/refactor to other family later on. Although It may take more effort overall, it will ensure we don't write more code than we need.

KarlK90 commented 2 years ago

Sorry for late response, after TET, I kind of forget about this.

No problem, hope you had a good TET :tada:

I will merge this first and make an PR for self-update afterwards. Thank you very much for your effort and patience on this PR.

I was pretty busy in the last weeks that is why I didn't respond earlier. Thanks for the merge and taking over from here! Happy that I could provide a good starting point for the self-update feature.

hathach commented 2 years ago

@KarlK90 no problem at all, I fully understand the busy with other works well enough. Btw, I struggled a bit with old cmsis f4, since its system_stm32f4xx.c reset the vector table (SCB->VTOR) to default 0x08000000 (incorrect). Just update to latest cmsis, the later skip statement if not defined

https://github.com/STMicroelectronics/cmsis_device_f4/blob/2615e866fa48fe1ff1af9e31c348813f2b19e7ec/Source/Templates/system_stm32f4xx.c#L181

I am putting thing up together, self-update may not be related to protected bootloader, but we must get them running together before enabling it. Otherwise we will have a chance to lock the bootloader which could require user to run various code to unlock/flash and risk bricking the board.

myst729 commented 2 years ago

It's frustrating when I try to erase tinyuf2 after flashed it. No tool provided in the first place. Follow tinyuf2-unlocker guide but the build artifact doesn't work at all. Try to disable write protection and erase via STM32Cube, the blackpill drive pops again. Any ideas?

myst729 commented 2 years ago

@hathach

default 0x08000000 (incorrect)

This is what I encounter now. Unfortunately I only have one CMSIS-DAP so I cannot update its firmware (which must be done from the host over another downloader). I found pyocd could erase listed sectors with --sector erase option:

pyocd erase -t stm32f401cdux  -s 0x08000000

Is this achievable, and which sector shall I pass?


pyocd flash -t stm32f401cdux dg_f401_default.bin                                                                                                   [21:29:55]
0001939:INFO:load_cmd:Loading /Users/leo/Downloads/dg_f401_default.bin
[                                                  ]   0%0007318:CRITICAL:__main__:flash erase sector failure (address 0x08000000; result code 0x1)
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/pyocd/__main__.py", line 150, in run
    status = cmd.invoke()
  File "/usr/local/lib/python3.9/site-packages/pyocd/subcommands/load_cmd.py", line 117, in invoke
    programmer.program(filename,
  File "/usr/local/lib/python3.9/site-packages/pyocd/flash/file_programmer.py", line 171, in program
    self._loader.commit()
  File "/usr/local/lib/python3.9/site-packages/pyocd/flash/loader.py", line 289, in commit
    perf = builder.program(chip_erase=chipErase,
  File "/usr/local/lib/python3.9/site-packages/pyocd/flash/builder.py", line 509, in program
    flash_operation = self._sector_erase_program_double_buffer(progress_cb)
  File "/usr/local/lib/python3.9/site-packages/pyocd/flash/builder.py", line 911, in _sector_erase_program_double_buffer
    self.flash.erase_sector(sector.addr)
  File "/usr/local/lib/python3.9/site-packages/pyocd/flash/flash.py", line 374, in erase_sector
    raise FlashEraseFailure('flash erase sector failure', address=address, result_code=result)
pyocd.core.exceptions.FlashEraseFailure: flash erase sector failure (address 0x08000000; result code 0x1)
hathach commented 2 years ago

@myst729 please open a new issue for your problem, this PR is merged and closed.