Add bootloader Firmware integrity check

[Olivier-ADLER]

We should check for firmware integrity before starting it. A CRC value could be stored in the Flash area.

[rmackay9]

Isn't there already a check when the firmware is loaded by the mission planner?

[Olivier-ADLER]

Yes but according to me it is not enough because we are dealing with flying machines. Flash memory corruption should be checked at boot. I have seen a user in the forum flying with a corrupted firmware ! (reported as corrupted by Mission Planner. The fimrware should not be loaded by the bootloader if it is corrupted. Too much dangerous.

[owenson]

Should be relatively simple to implement a CRC check into the bootloader. Of course, the Make file will need changing too - so add the CRC to the hex file

I agree it's a good idea - even though it is an edge case.

[gmorph]

Currently there is no checksum done of the FMU firmware. We do check the CRC of the IO firmware and we check it matches the CRC in RomFS. If it doesn't it tries to update to the correct firmware. Thanks, Grant.

[auturgy]

@tridge thoughts? Between compile and upload checks, there are already a couple of barriers in place. We'd need to identify what specific set of circumstances could result in a corrupted firmware being loaded, and that firmware still being armable. If such an edge case exists, it should be fixed. If it doesn't, this issue should be closed.

[WickedShell]

@auturgy this is still valid. The ChibiOS IO uploader maintains a failure boolean so we can actually track if we have a bad/unexpected CRC on the IO firmware. Flying with a mismatched IO/FMU firmware can be catastrophic and should be prevented from arming. Similarly I think there is still value on checking the over flash checksum at boot, as corruption here can have subtle effects.

[jmachuca77]

@tridge thoughts?

[tridge]

@WickedShell we now check the CRC on the IO firmware

[WickedShell]

Yeah, that is a good improvement. It would still be nice if the actual bootloader checksummed that the flashed image matches an expected value, but it probably requires a slight set of bootloader changes.

[peterbarker]

Where would the bootloader stash the correct value on firmware-flash?

Our new bootloader doesn't currently have support for BOOT_DELAY_ADDRESS, which would be the nearest thing I can think of.

[WickedShell]

@peterbarker it would have to stash it a predetermined location (such as last 4 bytes of flashed image) as part of the flashing process.

[peterbarker]

@WickedShell https://github.com/ardupilot/Bootloader/blob/master/bl.c#L657 <- that's the code I was refering to

[Naterater]

Does the new ChibiOS IO address this integrity check?

[peterbarker]

A suitably paranoid user can now achieve this using AP_CheckFirmware (check_good_firmware_unsigned).

I think we can close this one. Any objections?

[peterbarker]

No objections, closing this one now.