diff options
author | Julius Werner <jwerner@chromium.org> | 2020-10-15 17:37:57 -0700 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2021-03-17 08:10:00 +0000 |
commit | 81dc20e744aa1762c17dcf5aac5c37643d62a983 (patch) | |
tree | 1605652d540d384b6ea5b4ca585cba25d1a720d8 /util/cbfstool/cbfstool.c | |
parent | 2e973942bc34ff2d7b110ba35bf3cda987838907 (diff) | |
download | coreboot-81dc20e744aa1762c17dcf5aac5c37643d62a983.tar.gz coreboot-81dc20e744aa1762c17dcf5aac5c37643d62a983.tar.bz2 coreboot-81dc20e744aa1762c17dcf5aac5c37643d62a983.zip |
cbfs: Move stage header into a CBFS attribute
The CBFS stage header is part of the file data (not the header) from
CBFS's point of view, which is problematic for verification: in pre-RAM
environments, there's usually not enough scratch space in CBFS_CACHE to
load the full stage into memory, so it must be directly loaded into its
final destination. However, that destination is decided from reading the
stage header. There's no way we can verify the stage header without
loading the whole file and we can't load the file without trusting the
information in the stage header.
To solve this problem, this patch changes the CBFS stage format to move
the stage header out of the file contents and into a separate CBFS
attribute. Attributes are part of the metadata, so they have already
been verified before the file is loaded.
Since CBFS stages are generally only meant to be used by coreboot itself
and the coreboot build system builds cbfstool and all stages together in
one go, maintaining backwards-compatibility should not be necessary. An
older version of coreboot will build the old version of cbfstool and a
newer version of coreboot will build the new version of cbfstool before
using it to add stages to the final image, thus cbfstool and coreboot's
stage loader should stay in sync. This only causes problems when someone
stashes away a copy of cbfstool somewhere and later uses it to try to
extract stages from a coreboot image built from a different revision...
a debugging use-case that is hopefully rare enough that affected users
can manually deal with finding a matching version of cbfstool.
The SELF (payload) format, on the other hand, is designed to be used for
binaries outside of coreboot that may use independent build systems and
are more likely to be added with a potentially stale copy of cbfstool,
so it would be more problematic to make a similar change for SELFs. It
is not necessary for verification either, since they're usually only
used in post-RAM environments and selfload() already maps SELFs to
CBFS_CACHE before loading them to their final destination anyway (so
they can be hashed at that time).
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I8471ad7494b07599e24e82b81e507fcafbad808a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46484
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Diffstat (limited to 'util/cbfstool/cbfstool.c')
-rw-r--r-- | util/cbfstool/cbfstool.c | 74 |
1 files changed, 56 insertions, 18 deletions
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 3e80712ca221..6133536b4bed 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -14,7 +14,9 @@ #include "cbfs_sections.h" #include "elfparsing.h" #include "partitioned_file.h" +#include "lz4/lib/xxhash.h" #include <commonlib/bsd/cbfs_private.h> +#include <commonlib/bsd/compression.h> #include <commonlib/bsd/metadata_hash.h> #include <commonlib/fsp.h> #include <commonlib/endian.h> @@ -911,16 +913,7 @@ static int cbfs_add_component(const char *filename, sizeof(struct cbfs_file_attr_position)); if (attrs == NULL) goto error; - /* If we add a stage or a payload, we need to take */ - /* care about the additional metadata that is added */ - /* to the cbfs file and therefore set the position */ - /* the real beginning of the data. */ - if (type == CBFS_TYPE_STAGE) - attrs->position = htonl(offset - sizeof(struct cbfs_stage)); - else if (type == CBFS_TYPE_SELF) - attrs->position = htonl(offset - sizeof(struct cbfs_payload)); - else - attrs->position = htonl(offset); + attrs->position = htonl(offset); } /* Add alignment attribute if used */ if (param.alignment) { @@ -1118,11 +1111,18 @@ static int cbfstool_convert_mkstage(struct buffer *buffer, uint32_t *offset, * stages that would actually fit once compressed. */ if ((param.alignment || param.stage_xip) && - do_cbfs_locate(offset, sizeof(struct cbfs_stage), data_size)) { + do_cbfs_locate(offset, sizeof(struct cbfs_file_attr_stageheader), + data_size)) { ERROR("Could not find location for stage.\n"); return 1; } + struct cbfs_file_attr_stageheader *stageheader = (void *) + cbfs_add_file_attr(header, CBFS_FILE_ATTR_TAG_STAGEHEADER, + sizeof(struct cbfs_file_attr_stageheader)); + if (!stageheader) + return -1; + if (param.stage_xip) { /* * Ensure the address is a memory mapped one. This assumes @@ -1132,19 +1132,57 @@ static int cbfstool_convert_mkstage(struct buffer *buffer, uint32_t *offset, *offset = convert_addr_space(param.image_region, *offset); ret = parse_elf_to_xip_stage(buffer, &output, offset, - param.ignore_section); + param.ignore_section, + stageheader); } else { - ret = parse_elf_to_stage(buffer, &output, param.compression, - param.ignore_section); + ret = parse_elf_to_stage(buffer, &output, param.ignore_section, + stageheader); } - if (ret != 0) return -1; + + /* Store a hash of original uncompressed stage to compare later. */ + size_t decmp_size = buffer_size(&output); + uint32_t decmp_hash = XXH32(buffer_get(&output), decmp_size, 0); + + /* Chain to base conversion routine to handle compression. */ + ret = cbfstool_convert_raw(&output, offset, header); + if (ret != 0) + goto fail; + + /* Special care must be taken for LZ4-compressed stages that the BSS is + large enough to provide scratch space for in-place decompression. */ + if (!param.precompression && param.compression == CBFS_COMPRESS_LZ4) { + size_t memlen = ntohl(stageheader->memlen); + size_t compressed_size = buffer_size(&output); + uint8_t *compare_buffer = malloc(memlen); + uint8_t *start = compare_buffer + memlen - compressed_size; + if (!compare_buffer) { + ERROR("Out of memory\n"); + goto fail; + } + memcpy(start, buffer_get(&output), compressed_size); + ret = ulz4fn(start, compressed_size, compare_buffer, memlen); + if (ret == 0) { + ERROR("Not enough scratch space to decompress LZ4 in-place -- increase BSS size or disable compression!\n"); + free(compare_buffer); + goto fail; + } else if (ret != (int)decmp_size || + decmp_hash != XXH32(compare_buffer, decmp_size, 0)) { + ERROR("LZ4 compression BUG! Report to mailing list.\n"); + free(compare_buffer); + goto fail; + } + free(compare_buffer); + } + buffer_delete(buffer); - // Direct assign, no dupe. - memcpy(buffer, &output, sizeof(*buffer)); - header->len = htonl(output.size); + buffer_clone(buffer, &output); return 0; + +fail: + buffer_delete(&output); + return -1; } static int cbfstool_convert_mkpayload(struct buffer *buffer, |