diff options
author | Bill Richardson <wfrichar@chromium.org> | 2014-12-03 14:10:13 -0800 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2014-12-04 05:08:21 +0000 |
commit | b406c1064b3a37002b3d4d953a4932628408f25b (patch) | |
tree | 4856ba249a1ef333b771431d82115aadce99b8fd | |
parent | 64ef69c48da1cdd227b169accb5f576247cd8a89 (diff) | |
download | vboot-b406c1064b3a37002b3d4d953a4932628408f25b.tar.gz |
futility: Don't copy the entire kernel partition just to sign a blob
When re-signing a kernel partition and writing the result into a
new file, make sure we only emit the vblock and kernel blob
instead of creating a new file that's the size of the entire
partition.
Also add a test for that.
BUG=chromium:418647
BRANCH=none
TEST=make runtests
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Change-Id: I2c42aec6816e7e7abbeed360089c9b51fdcfe786
Reviewed-on: https://chromium-review.googlesource.com/233039
Reviewed-by: Randall Spangler <rspangler@chromium.org>
-rw-r--r-- | futility/cmd_sign.c | 63 | ||||
-rw-r--r-- | futility/misc.c | 6 | ||||
-rwxr-xr-x | tests/futility/test_sign_kernel.sh | 23 |
3 files changed, 61 insertions, 31 deletions
diff --git a/futility/cmd_sign.c b/futility/cmd_sign.c index c28aee99..f256b00e 100644 --- a/futility/cmd_sign.c +++ b/futility/cmd_sign.c @@ -302,17 +302,20 @@ int futil_cb_resign_kernel_part(struct futil_traverse_state_s *state) } Debug("vblock_size = 0x%" PRIx64 "\n", vblock_size); - if (option.vblockonly) { - /* If we're only writing the vblock, then we should be doing it - * into a new file. */ - rv = WriteSomeParts(option.outfile, - vblock_data, vblock_size, - NULL, 0); + if (option.create_new_outfile) { + /* Write out what we've been asked for */ + if (option.vblockonly) + rv = WriteSomeParts(option.outfile, + vblock_data, vblock_size, + NULL, 0); + else + rv = WriteSomeParts(option.outfile, + vblock_data, vblock_size, + kblob_data, kblob_size); } else { - /* If we're writing the whole thing, then the output is - * the same size (and possibly the same file) as the input. - * Either way, it's mmap'ed so modifications to the buffer - * will get flushed to disk when we close the file. */ + /* If we're modifying an existing file, it's mmap'ed so that + * all our modifications to the buffer will get flushed to + * disk when we close it. */ Memcpy(kpart_data, vblock_data, vblock_size); } @@ -879,6 +882,12 @@ static int do_sign(int argc, char *argv[]) } } + /* Look for an output file if we don't have one, just in case. */ + if (!option.outfile && argc - optind > 0) { + inout_file_count++; + option.outfile = argv[optind++]; + } + /* What are we looking at? */ type = futil_what_file_type(infile); @@ -891,6 +900,8 @@ static int do_sign(int argc, char *argv[]) type = FILE_TYPE_RAW_FIRMWARE; } + Debug("type=%s\n", futil_file_type_str[type]); + /* Check the arguments for the type of thing we want to sign */ switch (type) { case FILE_TYPE_UNKNOWN: @@ -942,7 +953,7 @@ static int do_sign(int argc, char *argv[]) break; case FILE_TYPE_KERN_PREAMBLE: errorcnt += no_opt_if(!option.signprivate, "signprivate"); - if (option.vblockonly) + if (option.vblockonly || inout_file_count > 1) option.create_new_outfile = 1; break; case FILE_TYPE_RAW_FIRMWARE: @@ -965,29 +976,21 @@ static int do_sign(int argc, char *argv[]) DIE; } - /* If we don't have an output file, we may need one */ + Debug("infile=%s\n", infile); + Debug("inout_file_count=%d\n", inout_file_count); + Debug("option.create_new_outfile=%d\n", option.create_new_outfile); + + /* Make sure we have an output file if one is needed */ if (!option.outfile) { - if (argc - optind > 0) { - /* We have an outfile arg, so use it. */ - inout_file_count++; - option.outfile = argv[optind++]; + if (option.create_new_outfile) { + errorcnt++; + fprintf(stderr, "Missing output filename\n"); + goto done; } else { - if (option.create_new_outfile) { - /* A distinct outfile is required */ - errorcnt++; - fprintf(stderr, "Missing output filename\n"); - goto done; - } else { - /* We'll just modify the input file */ - option.outfile = infile; - } + option.outfile = infile; } } - Debug("type=%d\n", type); - Debug("option.create_new_outfile=%d\n", option.create_new_outfile); - Debug("inout_file_count=%d\n", inout_file_count); - Debug("infile=%s\n", infile); Debug("option.outfile=%s\n", option.outfile); if (argc - optind > 0) { @@ -1005,6 +1008,7 @@ static int do_sign(int argc, char *argv[]) /* The input is read-only, the output is write-only. */ mapping = MAP_RO; state.in_filename = infile; + Debug("open RO %s\n", infile); ifd = open(infile, O_RDONLY); if (ifd < 0) { errorcnt++; @@ -1018,6 +1022,7 @@ static int do_sign(int argc, char *argv[]) state.in_filename = option.outfile; if (inout_file_count > 1) futil_copy_file_or_die(infile, option.outfile); + Debug("open RW %s\n", option.outfile); ifd = open(option.outfile, O_RDWR); if (ifd < 0) { errorcnt++; diff --git a/futility/misc.c b/futility/misc.c index 42e8bb44..4d8d762e 100644 --- a/futility/misc.c +++ b/futility/misc.c @@ -130,7 +130,7 @@ int print_hwid_digest(GoogleBinaryBlockHeader *gbb, uint8_t *buf = (uint8_t *)gbb; char *hwid_str = (char *)(buf + gbb->hwid_offset); int is_valid = 0; - uint8_t* digest = DigestBuf(buf + gbb->hwid_offset, + uint8_t *digest = DigestBuf(buf + gbb->hwid_offset, strlen(hwid_str), SHA256_DIGEST_ALGORITHM); if (digest) { @@ -159,7 +159,7 @@ void update_hwid_digest(GoogleBinaryBlockHeader *gbb) uint8_t *buf = (uint8_t *)gbb; char *hwid_str = (char *)(buf + gbb->hwid_offset); - uint8_t* digest = DigestBuf(buf + gbb->hwid_offset, + uint8_t *digest = DigestBuf(buf + gbb->hwid_offset, strlen(hwid_str), SHA256_DIGEST_ALGORITHM); memcpy(gbb->hwid_digest, digest, SHA256_DIGEST_SIZE); @@ -175,6 +175,8 @@ void futil_copy_file_or_die(const char *infile, const char *outfile) pid_t pid; int status; + Debug("%s(%s, %s)\n", __func__, infile, outfile); + pid = fork(); if (pid < 0) { diff --git a/tests/futility/test_sign_kernel.sh b/tests/futility/test_sign_kernel.sh index ebc50fc9..73da5ddb 100755 --- a/tests/futility/test_sign_kernel.sh +++ b/tests/futility/test_sign_kernel.sh @@ -271,6 +271,29 @@ try_arch () { # The rest of the partition should be unchanged. cmp -i ${blobsize} ${TMP}.part1.${arch} ${TMP}.part6.${arch}.new1 + # repack it the new way, from input to output + cp ${TMP}.part1.${arch} ${TMP}.part1.${arch}.in + ${FUTILITY} sign --debug \ + --signprivate ${DEVKEYS}/kernel_data_key.vbprivk \ + --keyblock ${DEVKEYS}/kernel.keyblock \ + --version 2 \ + --pad ${padding} \ + --config ${TMP}.config2.txt \ + --bootloader ${TMP}.bootloader2.bin \ + ${TMP}.part1.${arch}.in \ + ${TMP}.part6.${arch}.new2 + + ${FUTILITY} vbutil_kernel --verify ${TMP}.part6.${arch}.new2 \ + --pad ${padding} \ + --signpubkey ${DEVKEYS}/kernel_subkey.vbpubk > ${TMP}.verify6.new2 + + # The input file should not have changed (just being sure). + cmp ${TMP}.part1.${arch} ${TMP}.part1.${arch}.in + # The verification should be indentical + diff ${TMP}.verify6.old ${TMP}.verify6.new2 + # And creating a new output file should only emit a blob's worth + cmp ${TMP}.part6.${arch} ${TMP}.part6.${arch}.new2 + # Note: We specifically do not test repacking with a different --kloadaddr, # because the old way has a bug and does not update params->cmd_line_ptr to # point at the new on-disk location. Apparently (and not surprisingly), no |