ermtl / Open-Source-Ventilator

Complete control software for an emergency medical ventilator.
GNU General Public License v3.0
52 stars 30 forks source link

Canary memory / stack check. #7

Open ermtl opened 4 years ago

ermtl commented 4 years ago

To detect memory & stack corruption, a known good and easy way is to add canaries. For good protection, all the canaries should not have the same value and they should be spread among other variables and several of them (8seems reasonable) should be put, if possible at both ends of the stack and at both ends of the variables area. Any corruption detected by the checking function means something seriously wrong happened. If it happens, it should just write an error code to EEPROM and call a handler. Upon the next start, an error message should be displayed to alert the user that the machine could be unreliable.

the handler should decide what to do next. Usually activate an alarm and maybe stop. For a medical device, we should ask advice from a MD about the best option (stop or keep going with a serious risk).

An important point is to look at the resulting assembly code and check that the compiler did not "optimize" the canaries either by removing them, merging them or grouping them all together. also the stack canaries location relative to the variables and stack should be checked. This is a job for a compiler wizard !

Info about canaries: https://en.wikipedia.org/wiki/Buffer_overflow_protection#Canaries Info about stack overflows and stack canaries: https://en.wikipedia.org/wiki/Stack_buffer_overflow

This not being specific to the project, it could be done as a as a separate, independent project.

mattd3v commented 4 years ago

Could the use of more C++ STL data structures help manage overflows, without adding as much code?

Blimpyway commented 4 years ago

I worry more by not having a watchdog timer.

What do you mean by code corruption? AVRDUDE (or other flasher) does verify the code after it was written. Do you really think you (or something else like the program itself or an EMP) can change after it was flashed ?
If this was a thing somebody on either arduino or avrfreaks forum would freak about it.

wdt_example.zip

ermtl commented 4 years ago

mattd3v: Do you have an example of those structures being used on an Arduino ?

Blimpyway : The final code will have a watchdog timer. I did not add it for now as if people make mistakes in the code, it's easy to brick the board, and the program will need to be wiped out completely, reflashing the bootloader won't help. So people trying the code would lose their hardware (not everyone knows how to flash the entire chip).

If you look in my other repository, I worked on a version of the Adafruit_Sleepydog and added a mod that prevent bricking Chinese clones. I'll have to make it into a different library as Adafruit did not accept the pull. https://github.com/ermtl/Adafruit_SleepyDog

miguel5612 commented 4 years ago

ifdef Canary

disable sensorFeature and safe memory

endif

Same to arduino pro mini ATMEGA 168

ahogen commented 4 years ago

That watchdog example looks cool!

The GCC compiler flag -fstack-protector will call a void __stack_chk_fail(void) function if stack corruption is detected. Also the flag -fstack-limit-symbol=__StackLimit is interesting (where __StackLimit is a symbol from the linker script). Those options require modifying compiler flags, which isn't simple or convenient to do in the Arduino IDE.

The project could use a Makefile where the build arguments can be controlled by this project instead of the Arduino Foundation.