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>

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