trezor / trezor-firmware

:lock: Trezor Firmware Monorepo
https://trezor.io
Other
1.34k stars 653 forks source link

Removing SD card at runtime #856

Open matejcik opened 4 years ago

matejcik commented 4 years ago

The current SD card handling code in core/embed/trezorhal/sdcard.c assumes that the SD card is never removed while the relevant circuitry is powered on. This is a fair assumption, because the current TT hardware cannot handle the power fluctuation and it will reset.

In a future hardware revision, when this flaw is fixed, the code must be fixed as well.

When a new SD card is inserted, it must be initialized by calling some setup commands, so it is not enough to check if the circuitry is powered and a SD card is present, we also need to check whether the initialization was done.

Currently most commands check whether sd_handle.Instance is valid, i.e., the initialization function sdcard_power_on() was called. If the SD card is removed, the Instance will remain valid, despite there not being a SD card. In addition, if a new SD card is inserted, sdcard_is_present() will return true, but the newly inserted SD card will not have been initialized.

The check could be replaced by some sort of sdcard_was_initialized() which tries to query the SD card itself. Or an interrupt handler must detect the removal (by triggering when the relevant GPIO pin value changed) and call sdcard_power_off() to deactivate the circuitry and clear the sd_handle.Instance.

matejcik commented 4 years ago

the solution to this issue must also make sure to invalidate any existing FatFS instances

brianddk commented 2 years ago

The current SD card handling code in core/embed/trezorhal/sdcard.c assumes that the SD card is never removed while the relevant circuitry is powered on. This is a fair assumption, because the current TT hardware cannot handle the power fluctuation and it will reset.

I've noticed that hot-remove is no longer causing a reset (good), but will confirm that hot-add does hang the firmware (touch-screen non-responsive). How dangerous would you say hot-remove is to the TT device itself primarily, and the FatFS-SD secondarily.

I ask because my standard operational security is:

  1. Insert SD
  2. Plug in Trezor
  3. Unlock Trezor
  4. Remove SD
  5. Perform wallet functions
  6. Unplug Trezor

I get through operations #1 to #3 in less than a minute, but #4 could take several minutes or even an hour depending on checks and verifications. Since sd-protect is a theft deterrent I like the fact that someone rushing me and yanking the Trezor from my table will be of no use once #4 is done. Yes, I have a passphrase, but, again, the point of sd-protect was to be a better theft deterrent.

matejcik commented 2 years ago

How dangerous would you say hot-remove is to the TT device itself

It's not damaging. It should work fine in theory, because in case of unplug the power consumption goes down, so the voltage should remain stable. The SD card circuitry is turned off at most times, so there shouldn't even be glitches from that. In practice, who knows. If it works fine for you, that's as good as you're gonna get.

the FatFS-SD secondarily.

From the point of view of FatFS, this is safe unless Trezor is actually writing something to the SD card at that time. Unlike big computers, there is no "this operation will finish later", so as soon as Trezor is idle, you're good.

From my very limited understanding, SD cards should be OK with being unplugged at random times, so it shouldn't damage the card itself either.