7 comments:
File src/commonlib/bsd/include/commonlib/bsd/metadata_hash.h:
Patch Set #8, Line 14: hash above, which may be shorter than the compile-time size of struct vb2_hash! */
What's the reason for doing this as opposed to honoring the fixed length of the object itself?
Patch Set #8, Line 25: /* Must *not* appear anywhere else in coreboot code (other than the anchor). */
Wouldn't the act of verifying the magic value include the immediate value? Update: looks like we're not looking for this string in the code.
Maybe we should note that more explicitly. And if it's not supposed to be referenced anywhere in the code why provide a #define in a header file for people to obtain? For building purposes? vs runtime?
File src/include/metadata_hash.h:
Patch Set #8, Line 12: struct vb2_hash *metadata_hash_get(void);
could you please provide some commentary as to what this function would be used for and when?
Patch Set #8, Line 36: if (cbd != vboot_get_cbfs_boot_device()) {
I'm confused by this check. This error is only true when !NO_CBFS_MCACHE as well as !recovery. I see the Kconfig dependencies, but the code would be clearer if !NO_CBFS_MCACHE as added here as well. Separately, cbfs_boot_lookup() has force_ro parameter so it's not clear to me this is a valid check w/o checking force_ro here too.
Patch Set #8, Line 471: die("RO CBFS initialization error: %d", err);
I'm still having a hard time grok'ing why it's ok to fill up the mcache and not fail for Chrome OS. It makes it really hard to deal w/ checking integrity of the files' contents when the metadata is only sitting on boot media which means you'll have to re-walk the whole cbfs for a missing file lookup in mcache.
Patch Set #8, Line 37: ENV_BOOTBLOCK
ENV_INITIAL_STAGE?
Patch Set #8, Line 10: struct metadata_hash_anchor metadata_hash_anchor = {
why is this global? and not const?
To view, visit change 41120. To unsubscribe, or for help writing mail filters, visit settings.