Avoiding accidental firmware overwrite

Question

First some background. When firmware for whatever reason crashes (e.g. stack overflow, corrupted function pointer...) it may happen, that it jumps somewhere and starts executing some code. This will sooner or later result in watchdog reset. MCU will reset and we are back on track. Unless...

What about when we have code that writes to flash (e.g. bootloader)? Now it can happen that we will accidentally jump directly into the flash write code - skipping all the checks. Before watchdog will bark you will end up with corrupted firmware. This is exactly what was happening to me.

Now some might say - fix the root bug that caused that we even jumped into write code. Well, when you are developing you are constantly changing the code. Even if there is no such bug in there at the moment, there might be tomorrow. Besides, no code is bug free - or at least not mine.

So now I am doing some kind of cross checking. I have a variable named 'wen' which I set to 0xa5 before the usual checks (e.g. check to make sure that destination is valid). Then just before doing the actual erase or write I check if 'wen' is really set to 0xa5. Otherwise this means that we somehow accidentally jumped into the writing code. After successful write 'wen' is cleared. I have done this in C and it worked well. But there is still slight theoretical chance corruption will happen, cause there are few instructions from this final check of 'wen' till write to SPMCR register.

Now I want to improve this by putting this check into assembly, between the write to SPMCR and spm instruction.

__asm__ __volatile__
(   
    "lds __zero_reg__, %0\n\t"
    "out %1, %2\n\t"
    "ldi r25, %3\n\t"
    "add __zero_reg__, r25\n\t"
    "brne spm_fail\n\t"
    "spm\n\t"
    "rjmp spm_done\n\t"
    "spm_fail: clr __zero_reg__\n\t"
    "call __assert\n\t"
    "spm_done:"
    :
    : "i" ((uint16_t)(&wen)),
      "I" (_SFR_IO_ADDR(__SPM_REG)),
      "r" ((uint8_t)(__BOOT_PAGE_ERASE)),
      "M" ((uint8_t)(-ACK)),
      "z" ((uint16_t)(adr))
   : "r25"
);

Haven't tried the code yet, will do that tomorrow. Do you see any problems? How do/would you solve such problem?

Answer 1

One technique I've seen is to make sure the bytes immediately before your flash write routines will trigger some sort of watchdog timeout, or reset the processor. That way, it's not possible to execute random data leading up to the flash write function and just "fall into" the function.

You may need to have some NOPs before your reset to ensure the instructions are interpreted correctly.

Your technique of verifying that the function has run from the beginning, looks like a good one, assuming you clear the wen variable once you've done the write.

Answer 2

Im not sure why you need to have the capability to write to flash in your bootloader. Our bootloader does, because it can update the application program via serial port. So we eliminate the potential for inadvertent writes by ensuring that the loader does not contain any of the code that writes to flash. That code is downloaded is a header in the same package that contains the image to be written. The onboard image has the checksum of the programming algo stored, and verifies it before running it.

If you are writing things that are generated internally, then I would look at hardware related interlocks. Only allow writes if you have previously set a particular discrete output pin to ON. To answer the problem of "what if the IP jumps past the checks"? you can do it in 2 parts. First set some critical variables for the algorithm. (the address to write to for example- keep that initialized to invalid memory, and only set it correctly in a separate call made before the write. Then have the write function check your HW interlock. Do one of the enable steps in an interrupt, or in response to a timer, something that is unlikely to be hit in the correct sequence if you have a rogue IP.

If your IP can really jump anywhere , it may be impossible to prevent an inadvertent write. The best you can hope for is that you ensure the only path to get there also sets up everything else needed for a successful write.