Major refactoring of vbutil_kernel

This started out as a simple fix for a minor bug and turned into a nearly
complete rewrite. Now that it's done I'm not sure it really matters. This
version is a lot cleaner about handling command-line args, but isn't
otherwise noticeably better. Sigh.

BUG=none
TEST=manual

make
make runtests

Change-Id: I9c194e9c0e6418488635989ef666bc83c6e39816
Reviewed-on: https://gerrit.chromium.org/gerrit/18268
Commit-Ready: Bill Richardson <wfrichar@chromium.org>
Tested-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

host/lib/host_misc.c

@@ -24,9 +24,10 @@ char* StrCopy(char* dest, const char* src, int dest_size) {
 }
 
 
-uint8_t* ReadFile(const char* filename, uint64_t* size) {
+uint8_t* ReadFile(const char* filename, uint64_t* sizeptr) {
   FILE* f;
   uint8_t* buf;
+  uint64_t size;
 
   f = fopen(filename, "rb");
   if (!f) {
@@ -35,16 +36,16 @@ uint8_t* ReadFile(const char* filename, uint64_t* size) {
   }
 
   fseek(f, 0, SEEK_END);
-  *size = ftell(f);
+  size = ftell(f);
   rewind(f);
 
-  buf = malloc(*size);
+  buf = malloc(size);
   if (!buf) {
     fclose(f);
     return NULL;
   }
 
-  if(1 != fread(buf, *size, 1, f)) {
+  if(1 != fread(buf, size, 1, f)) {
     VBDEBUG(("Unable to read from file %s\n", filename));
     fclose(f);
     free(buf);
@@ -52,6 +53,8 @@ uint8_t* ReadFile(const char* filename, uint64_t* size) {
   }
 
   fclose(f);
+  if (sizeptr)
+    *sizeptr = size;
   return buf;
 }
 

tests/run_vbutil_kernel_arg_tests.sh

@@ -12,6 +12,7 @@
 . "$(dirname "$0")/common.sh"
 
 # directories
+DEVKEYS="${ROOT_DIR}/tests/devkeys"
 DATA_DIR="${SCRIPT_DIR}/preamble_tests/data"
 TMPDIR="${TEST_DIR}/vbutil_kernel_arg_tests_dir"
 [ -d "${TMPDIR}" ] || mkdir -p "${TMPDIR}"
@@ -43,7 +44,7 @@ while [ "$k" -lt "${#KERN_VALS[*]}" ]; do
   while [ "$b" -lt "${#BOOT_VALS[*]}" ]; do
     echo -n "pack kern_${k}_${b}.vblock ... "
     : $(( tests++ ))
-      ${UTIL_DIR}/vbutil_kernel --pack "${TMPDIR}/kern_${k}_${b}.vblock" \
+      "${UTIL_DIR}/vbutil_kernel" --pack "${TMPDIR}/kern_${k}_${b}.vblock" \
         --keyblock "${KEYBLOCK}" \
         --signprivate "${SIGNPRIVATE}" \
         --version 1 \
@@ -86,6 +87,109 @@ for v in ${TMPDIR}/kern_*.vblock; do
   fi
 done
 
+
+
+# Test repacking a USB image for the SSD, the way the installer does.
+
+set -e
+# Pack for USB
+USB_KERN="${TMPDIR}/usb_kern.bin"
+USB_KEYBLOCK="${DEVKEYS}/recovery_kernel.keyblock"
+USB_SIGNPRIVATE="${DEVKEYS}/recovery_kernel_data_key.vbprivk"
+USB_SIGNPUBKEY="${DEVKEYS}/recovery_key.vbpubk"
+echo -n "pack USB kernel ... "
+: $(( tests++ ))
+"${UTIL_DIR}/vbutil_kernel" \
+  --pack "${USB_KERN}" \
+  --keyblock "${USB_KEYBLOCK}" \
+  --signprivate "${USB_SIGNPRIVATE}" \
+  --version 1 \
+  --config "${CONFIG}" \
+  --bootloader "${BIG}" \
+  --vmlinuz "${BIG}" \
+  --arch arm
+if [ "$?" -ne 0 ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
+# And verify it.
+echo -n "verify USB kernel ... "
+: $(( tests++ ))
+"${UTIL_DIR}/vbutil_kernel" \
+  --verify "${USB_KERN}" \
+  --signpubkey "${USB_SIGNPUBKEY}" >/dev/null
+if [ "$?" -ne 0 ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
+# Now we re-sign the same image using the normal keys. This is the kernel
+# image that is put on the hard disk by the installer. Note: To save space on
+# the USB image, we're only emitting the new verfication block, and the
+# installer just replaces that part of the hard disk's kernel partition.
+SSD_KERN="${TMPDIR}/ssd_kern.bin"
+SSD_KEYBLOCK="${DEVKEYS}/kernel.keyblock"
+SSD_SIGNPRIVATE="${DEVKEYS}/kernel_data_key.vbprivk"
+SSD_SIGNPUBKEY="${DEVKEYS}/kernel_subkey.vbpubk"
+echo -n "repack to SSD kernel ... "
+: $(( tests++ ))
+"${UTIL_DIR}/vbutil_kernel" \
+  --repack "${SSD_KERN}" \
+  --vblockonly \
+  --keyblock "${SSD_KEYBLOCK}" \
+  --signprivate "${SSD_SIGNPRIVATE}" \
+  --oldblob "${TMPDIR}/usb_kern.bin" >/dev/null
+if [ "$?" -ne 0 ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
+# To verify it, we have to replace the vblock from the original image.
+tempfile="${TMPDIR}/foo.bin"
+cat "${SSD_KERN}" > "$tempfile"
+dd if="${USB_KERN}" bs=65536 skip=1 >> $tempfile 2>/dev/null
+
+echo -n "verify SSD kernel ... "
+: $(( tests++ ))
+"${UTIL_DIR}/vbutil_kernel" \
+  --verify "$tempfile" \
+  --signpubkey "${SSD_SIGNPUBKEY}" >/dev/null
+if [ "$?" -ne 0 ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
+# Finally make sure that the kernel command line stays good.
+orig=$(cat "${CONFIG}" | tr '\012' ' ')
+packed=$("${UTIL_DIR}/dump_kernel_config" "${USB_KERN}")
+echo -n "check USB kernel config ..."
+: $(( tests++ ))
+if [ "$orig" != "$packed" ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
+repacked=$("${UTIL_DIR}/dump_kernel_config" "${tempfile}")
+echo -n "check SSD kernel config ..."
+: $(( tests++ ))
+if [ "$orig" != "$packed" ]; then
+  echo -e "${COL_RED}FAILED${COL_STOP}"
+  : $(( errs++ ))
+else
+  echo -e "${COL_GREEN}PASSED${COL_STOP}"
+fi
+
 # Summary
 ME=$(basename "$0")
 if [ "$errs" -ne 0 ]; then

utility/dump_kernel_config.c

@@ -38,22 +38,16 @@ uint8_t* find_kernel_config(uint8_t* blob, uint64_t blob_size,
     return NULL;
   }
 
-  /* The parameters are packed before the bootloader and there is no specific
-   * pointer to it so we just walk back by its allocated size. */
+  /* The x86 kernels have a pointer to the kernel commandline in the zeropage
+   * table, but that's irrelevant for ARM. Both types keep the config blob in
+   * the same place, so just go find it. */
   offset = preamble->bootloader_address -
-           (kernel_body_load_address + CROS_PARAMS_SIZE) + now;
+    (kernel_body_load_address + CROS_PARAMS_SIZE +
+     CROS_CONFIG_SIZE) + now;
   if (offset > blob_size) {
     VbExError("params are outside of the memory blob: %x\n", offset);
     return NULL;
   }
-  params = (struct linux_kernel_params *)(blob + offset);
-
-  /* Grab the offset to the kernel command line using the supplied pointer. */
-  offset = params->cmd_line_ptr - kernel_body_load_address + now;
-  if (offset > blob_size) {
-    VbExError("cmdline is outside of the memory blob: %x\n", offset);
-    return NULL;
-  }
   return blob + offset;
 }