[U-Boot] [PATCH] spl: implement CRC check on U-Boot uImage

[Simon Goldschmidt]

On 27.11.2018 15:57, Wolfgang Denk wrote:
> Dear Simon,
>
> In message <20181124124641.6605-1-simon.k.r.goldschmidt at gmail.com> you wrote:
>> SPL currently does not check uImage CRCs when loading U-Boot.
>>
>> This patch adds checking the uImage CRC when SPL loads U-Boot. It does
>> this by reusing the existing config option SPL_CRC32_SUPPORT to allow
>> leaving out the CRC check on boards where the additional code size or
>> boot time is a problem (adding the CRC check currently adds ~1.4 kByte
>> to flash).
>>
>> The SPL_CRC32_SUPPORT config option now gets enabled by default if SPL
>> support for legacy images is enabled to check the CRC on all boards
>> that don't actively take countermeasures.
> The new features is definitive useful, but I have a few comments.
>
> First, I am not sure if mandatorily binding this feature to
> SPL_CRC32_SUPPORT is a good idea?
>
> Might there not be reasons to have SPL_CRC32_SUPPORT enabled but
> still wanting to keep the boot time minimal, and to intentionally NOT
> check the CRC (similar to what setting "verify=no" would do)?
> [This is commonly used when trading boot speed for reliability/securi-
> ty...]

I also thought about that. I voted against to have less config options. 
But sure, I can add a config option for this in v2.

> Second, may I please ask you to change the code such that the CRC is
> checked "in-flight", i. e. while reading from the boot device and
> writing it to the RAM, instead of re-reading the image from RAM?
> Again, the reason is boot time. We will not reach a zero-copy boot,
> but at least we should avoid an additional reading of what we just
> wrote.  Yes, this would catch memory corruptions in RAM, but the
> whole operation of U-Boot is based on the assumption that RAM is
> working reliable and error-free. [If you drop this assumption, you
> cannot guaranteee anything, i. e. you would have to fear that the
> memory content might even get corrupted when reading it for the CRC
> calculation - if you have such concerns, you should switch to
> hardware with ECC RAM.]

Hmm, while this is a good idea regarding speed, I'm not sure this can be 
done easily with the current infrastructure. The data is copied down at 
the driver somewhere. That would mean we'd have to change every driver 
interface and every driver to allow a callback where we'd update the CRC.

Is this really a rework that is worth it? Because it's also not done 
like that for loading other uImages (Kernel, fdt, ramdisk) when U-Boot 
loads the OS, or is it?

Regards,
Simon

>
> Thanks!
>
> Best regards,
>
> Wolfgang Denk
>