Attention is currently required from: Martin Roth, Furquan Shaikh, Aaron Durbin. Hello Furquan Shaikh, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/52085
to review the following change.
Change subject: decompressor: Add CBFS_VERIFICATION support ......................................................................
decompressor: Add CBFS_VERIFICATION support
CBFS_VERIFICATION requires the CBFS metadata hash anchor to be linked into an uncompressed stage, but for platforms using COMPRESS_BOOTBLOCK, this is only the decompressor stage. The first CBFS accesses are made in the bootblock stage after decompression, so if we want to make CBFS_VERIFICATION work on those platforms, we have to pass the metadata hash anchor from the decompressor into the bootblock. This patch does just that. (Note that this relies on the decompressor data remaining valid in memory for as long as the metadata hash anchor is needed. This is always true even for OVERLAP_DECOMPRESSOR_ROMSTAGE() situations because the FMAP and CBFS metadata necessarily need to have finished verification before a new stage could be loaded.)
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I2e6d7384cfb8339a24369eb6c01fc12f911c974e --- M src/include/bootblock_common.h M src/include/metadata_hash.h M src/lib/Kconfig.cbfs_verification M src/lib/Makefile.inc M src/lib/bootblock.c M src/lib/decompressor.c M src/lib/metadata_hash.c 7 files changed, 33 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/52085/1
diff --git a/src/include/bootblock_common.h b/src/include/bootblock_common.h index da627d2..fccd235 100644 --- a/src/include/bootblock_common.h +++ b/src/include/bootblock_common.h @@ -35,6 +35,7 @@ /* This is the argument structure passed from decompressor to bootblock. */ struct bootblock_arg { uint64_t base_timestamp; + void *metadata_hash_anchor; uint32_t num_timestamps; struct timestamp_entry timestamps[]; }; diff --git a/src/include/metadata_hash.h b/src/include/metadata_hash.h index 2d3b8a8..bfa7ef1 100644 --- a/src/include/metadata_hash.h +++ b/src/include/metadata_hash.h @@ -6,6 +6,11 @@
#include <commonlib/bsd/metadata_hash.h>
+/* Return a pointer to the whole anchor. Only used for decompressor builds. */ +void *metadata_hash_export_anchor(void); +/* Import a pointer that points to the anchor. Only used for decompressor builds. */ +void metadata_hash_import_anchor(void *ptr); + /* Verify the an FMAP data structure with the FMAP hash that is stored together with the CBFS metadata hash in the bootblock's metadata hash anchor (when CBFS verification is enabled). */ vb2_error_t metadata_hash_verify_fmap(const void *fmap_base, size_t fmap_size); diff --git a/src/lib/Kconfig.cbfs_verification b/src/lib/Kconfig.cbfs_verification index a28df1f..fa90d9d 100644 --- a/src/lib/Kconfig.cbfs_verification +++ b/src/lib/Kconfig.cbfs_verification @@ -6,7 +6,6 @@
config CBFS_VERIFICATION bool # TODO: make user selectable once it works - depends on !COMPRESS_BOOTBLOCK # TODO: figure out decompressor anchor depends on !VBOOT_STARTS_BEFORE_BOOTBLOCK # this is gonna get tricky... select VBOOT_LIB help diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 074cb2a..358035d 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -38,6 +38,7 @@ decompressor-$(CONFIG_GENERIC_GPIO_LIB) += gpio.c decompressor-y += memchr.c decompressor-y += memcmp.c +decompressor-$(CONFIG_CBFS_VERIFICATION) += metadata_hash.c decompressor-y += prog_ops.c decompressor-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c
diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c index 1509c8c..23fb392 100644 --- a/src/lib/bootblock.c +++ b/src/lib/bootblock.c @@ -4,6 +4,7 @@ #include <bootblock_common.h> #include <console/console.h> #include <delay.h> +#include <metadata_hash.h> #include <option.h> #include <post.h> #include <program_loading.h> @@ -88,6 +89,8 @@ void _start(struct bootblock_arg *arg); void _start(struct bootblock_arg *arg) { + if (CONFIG(CBFS_VERIFICATION)) + metadata_hash_import_anchor(arg->metadata_hash_anchor); bootblock_main_with_timestamp(arg->base_timestamp, arg->timestamps, arg->num_timestamps); } diff --git a/src/lib/decompressor.c b/src/lib/decompressor.c index 8ae9358..1d160e0 100644 --- a/src/lib/decompressor.c +++ b/src/lib/decompressor.c @@ -3,6 +3,7 @@ #include <bootblock_common.h> #include <commonlib/bsd/compression.h> #include <delay.h> +#include <metadata_hash.h> #include <program_loading.h> #include <symbols.h> #include <timestamp.h> @@ -42,6 +43,9 @@ if (CONFIG(COLLECT_TIMESTAMPS)) arg.base_timestamp = timestamp_get();
+ if (CONFIG(CBFS_VERIFICATION)) + arg.metadata_hash_anchor = metadata_hash_export_anchor(); + decompressor_soc_init();
if (CONFIG(COLLECT_TIMESTAMPS)) diff --git a/src/lib/metadata_hash.c b/src/lib/metadata_hash.c index a823c5f..a4a4b9e 100644 --- a/src/lib/metadata_hash.c +++ b/src/lib/metadata_hash.c @@ -5,6 +5,7 @@ #include <metadata_hash.h> #include <symbols.h>
+#if !CONFIG(COMPRESS_BOOTBLOCK) || ENV_DECOMPRESSOR __attribute__((used, section(".metadata_hash_anchor"))) static struct metadata_hash_anchor metadata_hash_anchor = { /* This is the only place in all of coreboot where we actually need to use this. */ @@ -12,15 +13,30 @@ .cbfs_hash = { .algo = CONFIG_CBFS_HASH_ALGO } };
+static struct metadata_hash_anchor *anchor_ptr = &metadata_hash_anchor; + +void *metadata_hash_export_anchor(void) +{ + return anchor_ptr; +} +#else +static struct metadata_hash_anchor *anchor_ptr = NULL; + +void metadata_hash_import_anchor(void *ptr) +{ + anchor_ptr = ptr; +} +#endif + struct vb2_hash *metadata_hash_get(void) { - return &metadata_hash_anchor.cbfs_hash; + return &anchor_ptr->cbfs_hash; }
vb2_error_t metadata_hash_verify_fmap(const void *fmap_buffer, size_t fmap_size) { - struct vb2_hash hash = { .algo = metadata_hash_anchor.cbfs_hash.algo }; - memcpy(hash.raw, metadata_hash_anchor_fmap_hash(&metadata_hash_anchor), + struct vb2_hash hash = { .algo = anchor_ptr->cbfs_hash.algo }; + memcpy(hash.raw, metadata_hash_anchor_fmap_hash(anchor_ptr), vb2_digest_size(hash.algo)); return vb2_hash_verify(fmap_buffer, fmap_size, &hash); }