Hello Patrick Georgi, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41119
to review the following change.
Change subject: cbfstool: Hide hash printing behind -v and add to parseable output ......................................................................
cbfstool: Hide hash printing behind -v and add to parseable output
With the upcoming introduction of CBFS verification, a lot more CBFS files will have hashes. The current cbfstool default of always printing hash attributes when they exist will make cbfstool print very messy. Therefore, hide hash attribute output unless the user passed -v.
It would also be useful to be able to get file attributes like hashes in machine parseable output. Unfortunately, our machine parseable format (-k) doesn't really seem designed to be extensible. To avoid breaking older parsers, this patch adds new attribute output behind -v (which hopefully no current users pass since it doesn't change anything for -k at the moment). With this patch cbfstool print -k -v may print an arbitrary amount of extra tokens behind the predefined ones on a file line. Tokens always begin with an identifying string (e.g. 'hash'), followed by extra fields that should be separated by colons. Multiple tokens are separated by the normal separator character (tab).
cbfstool print -k -v may also print additional information that applies to the whole CBFS on separate lines. These lines will always begin with a '[' (which hopefully nobody would use as a CBFS filename character although we technically have no restrictions at the moment).
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I9e16cda393fa0bc1d8734d4b699e30e2ae99a36d --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c 3 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/41119/1
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 7942b27..3a83694 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -1503,6 +1503,9 @@ decompressed_size );
+ if (!verbose) + return 0; + struct cbfs_file_attr_hash *attr = NULL; while ((attr = cbfs_file_get_next_hash(entry, attr)) != NULL) { size_t hash_len = vb2_digest_size(attr->hash.algo); @@ -1522,9 +1525,6 @@ free(hash_str); }
- if (!verbose) - return 0; - DEBUG(" cbfs_file=0x%x, offset=0x%x, content_address=0x%x+0x%x\n", cbfs_get_entry_addr(image, entry), ntohl(entry->offset), cbfs_get_entry_addr(image, entry) + ntohl(entry->offset), @@ -1587,21 +1587,45 @@ fprintf(fp, "%s%s", type, sep); fprintf(fp, "0x%zx%s", metadata_size, sep); fprintf(fp, "0x%zx%s", data_size, sep); - fprintf(fp, "0x%zx\n", metadata_size + data_size); + fprintf(fp, "0x%zx", metadata_size + data_size); + + if (verbose) { + unsigned int decompressed_size = 0; + unsigned int compression = cbfs_file_get_compression_info(entry, + &decompressed_size); + if (compression != CBFS_COMPRESS_NONE) + fprintf(fp, "%scomp:%s:0x%x", sep, lookup_name_by_type( + types_cbfs_compression, compression, "????"), + decompressed_size); + + struct cbfs_file_attr_hash *attr = NULL; + while ((attr = cbfs_file_get_next_hash(entry, attr)) != NULL) { + size_t hash_len = vb2_digest_size(attr->hash.algo); + if (!hash_len) + continue; + char *hash_str = bintohex(attr->hash.raw, hash_len); + int valid = vb2_hash_verify(CBFS_SUBHEADER(entry), + ntohl(entry->len), &attr->hash) == VB2_SUCCESS; + fprintf(fp, "%shash:%s:%s:%s", sep, + vb2_get_hash_algorithm_name(attr->hash.algo), + hash_str, valid ? "valid" : "invalid"); + free(hash_str); + } + } + fprintf(fp, "\n");
return 0; }
-int cbfs_print_directory(struct cbfs_image *image) +void cbfs_print_directory(struct cbfs_image *image) { if (cbfs_is_legacy_cbfs(image)) cbfs_print_header_info(image); printf("%-30s %-10s %-12s Size Comp\n", "Name", "Offset", "Type"); cbfs_legacy_walk(image, cbfs_print_entry_info, NULL); - return 0; }
-int cbfs_print_parseable_directory(struct cbfs_image *image) +void cbfs_print_parseable_directory(struct cbfs_image *image) { size_t i; const char *header[] = { @@ -1618,7 +1642,6 @@ fprintf(stdout, "%s%s", header[i], sep); fprintf(stdout, "%s\n", header[i]); cbfs_legacy_walk(image, cbfs_print_parseable_entry_info, stdout); - return 0; }
int cbfs_merge_empty_entry(struct cbfs_image *image, struct cbfs_file *entry, diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index a3a6fb9..d633fee 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -174,8 +174,8 @@ int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry);
/* Print CBFS component information. */ -int cbfs_print_directory(struct cbfs_image *image); -int cbfs_print_parseable_directory(struct cbfs_image *image); +void cbfs_print_directory(struct cbfs_image *image); +void cbfs_print_parseable_directory(struct cbfs_image *image); int cbfs_print_header_info(struct cbfs_image *image); int cbfs_print_entry_info(struct cbfs_image *image, struct cbfs_file *entry, void *arg); diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index f97745e..6015603 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1087,12 +1087,16 @@ if (cbfs_image_from_buffer(&image, param.image_region, param.headeroffset)) return 1; - if (param.machine_parseable) - return cbfs_print_parseable_directory(&image); - else { + if (param.machine_parseable) { + if (verbose) + printf("[FMAP REGION]\t%s\n", param.region_name); + cbfs_print_parseable_directory(&image); + } else { printf("FMAP REGION: %s\n", param.region_name); - return cbfs_print_directory(&image); + cbfs_print_directory(&image); } + + return 0; }
static int cbfs_extract(void)