On 27.11.2018 15:57, Wolfgang Denk wrote:
Dear Simon,
In message <20181124124641.6605-1-simon.k.r.goldschm...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
