Remove obsolete VerifyKernelDriver() and associated data structures.

The kernel rollback test needs to be rebaselined to use the LoadKernel() (or may combined with load_kernel_test). Will do that as a separate CL when that is close to its final form.

Review URL: http://codereview.chromium.org/2584001

tests/kernel_rollback_tests.c

@@ -23,6 +23,9 @@ void VerifyKernelDriverTest(void) {
   uint64_t len;
   uint8_t* firmware_key_pub = BufferFromFile(kFirmwareKeyPublicFile, &len);
 
+  /* TODO(gauravsh): Rebase this to use LoadKernel() (maybe by making
+   * it a part of load_kernel_test.c */
+#if 0
   /* Initialize kernel blobs, including their associated parition
    * table attributed. */
   kernel_entry valid_kernelA =  {
@@ -134,12 +137,13 @@ void VerifyKernelDriverTest(void) {
           BOOT_KERNEL_A_CONTINUE,
           "(Valid Kernel A (new version)\n"
           " Valid Kernel B (old version) runs A):");
-
-  Free(firmware_key_pub);
   Free(valid_kernelA.kernel_blob);
   Free(valid_kernelB.kernel_blob);
   Free(corrupt_kernelA.kernel_blob);
   Free(corrupt_kernelB.kernel_blob);
+#endif
+
+  Free(firmware_key_pub);
 }
 
 int main(int argc, char* argv[]) {

vboot_firmware/include/kernel_image_fw.h

@@ -164,38 +164,4 @@ int VerifyKernel(const uint8_t* signing_key_blob,
  * (kernel_key_version << 16 | kernel_version). */
 uint32_t GetLogicalKernelVersion(uint8_t* kernel_blob);
 
-#define  BOOT_KERNEL_A_CONTINUE 1
-#define  BOOT_KERNEL_B_CONTINUE 2
-#define  BOOT_KERNEL_RECOVERY_CONTINUE 3
-
-/* Contains information about the kernel paritition
- * gleaned from the GPT partition table.
- *
- * Based on the Chromium OS Drive Map design document by
- * rspangler@chromium.org.
- *
-*/
-typedef struct kernel_entry {
-  uint8_t* kernel_blob;  /* Pointer to actual kernel. */
-  uint8_t boot_priority;  /* 15 = highest, 1 = lowest, 0 = not bootable. */
-  uint8_t boot_tries_remaining;  /* Used when boot_priority = 0. */
-  uint8_t boot_success_flag;  /* Set to 1 on successful boot by AU. */
-} kernel_entry;
-
-/* This function is the driver used by the RW firmware to
- * determine which copy of the kernel to boot from. It performs
- * the requisite priority and remaining tries checking for a specific
- * kernel partition, does rollback index checking, including updating
- * if required.
- *
- * Returns the code path to follow. It is one of:
- * BOOT_KERNEL_A_CONTINUE          Boot from Kenrel A
- * BOOT_KERNEL_B_CONTINUE          Boot from Kernel B
- * BOOT_KERNEL_RECOVERY_CONTINUE   Jump to recovery mode
- */
-int VerifyKernelDriver_f(uint8_t* firmware_key_blob,
-                         kernel_entry* kernelA,
-                         kernel_entry* kernelB,
-                         int dev_mode);
-
 #endif  /* VBOOT_REFERENCE_KERNEL_IMAGE_FW_H_ */

vboot_firmware/lib/kernel_image_fw.c

@@ -381,109 +381,3 @@ uint32_t GetLogicalKernelVersion(uint8_t* kernel_blob) {
   Memcpy(&kernel_version, kernel_ptr, sizeof(kernel_version));
   return CombineUint16Pair(kernel_key_version, kernel_version);
 }
-
-int VerifyKernelDriver_f(uint8_t* firmware_key_blob,
-                         kernel_entry* kernelA,
-                         kernel_entry* kernelB,
-                         int dev_mode) {
-  int i;
-  /* Contains the logical kernel version (32-bit) which is calculated as
-   * (kernel_key_version << 16 | kernel_version) where
-   * [kernel_key_version], [firmware_version] are both 16-bit.
-   */
-  uint32_t kernelA_lversion, kernelB_lversion;
-  uint32_t min_lversion;  /* Minimum of kernel A and kernel B lversion. */
-  uint32_t stored_lversion;  /* Stored logical version in the TPM. */
-  kernel_entry* try_kernel[2];  /* Kernel in try order. */
-  int try_kernel_which[2];  /* Which corresponding kernel in the try order */
-  uint32_t try_kernel_lversion[2];  /* Their logical versions. */
-  uint16_t kernel_version, kernel_key_version;  /* Temporary variables */
-
-  /* [kernel_to_boot] will eventually contain the boot path to follow
-   * and is returned to the caller. Initially, we set it to recovery. If
-   * a valid bootable kernel is found, it will be set to that. */
-  int kernel_to_boot = BOOT_KERNEL_RECOVERY_CONTINUE;
-
-
-  /* The TPM must already have be initialized, so no need to call SetupTPM(). */
-
-  /* We get the key versions by reading directly from the image blobs without
-   * any additional (expensive) sanity checking on the blob since it's faster to
-   * outright reject a kernel with an older kernel key version. A malformed
-   * or corrupted kernel blob will still fail when VerifyKernel() is called
-   * on it.
-   */
-  kernelA_lversion = GetLogicalKernelVersion(kernelA->kernel_blob);
-  kernelB_lversion = GetLogicalKernelVersion(kernelB->kernel_blob);
-  min_lversion  = Min(kernelA_lversion, kernelB_lversion);
-  GetStoredVersions(KERNEL_VERSIONS, &kernel_key_version, &kernel_version);
-  stored_lversion = CombineUint16Pair(kernel_key_version, kernel_version);
-
-  /* TODO(gauravsh): The kernel entries kernelA and kernelB come from the
-   * partition table - verify its signature/checksum before proceeding
-   * further. */
-
-  /* The logic for deciding which kernel to boot from is taken from the
-   * the Chromium OS Drive Map design document.
-   *
-   * We went to consider the kernels in their according to their boot
-   * priority attribute value.
-   */
-
-  if (kernelA->boot_priority >= kernelB->boot_priority) {
-    try_kernel[0] = kernelA;
-    try_kernel_which[0] = BOOT_KERNEL_A_CONTINUE;
-    try_kernel_lversion[0] = kernelA_lversion;
-    try_kernel[1] = kernelB;
-    try_kernel_which[1] = BOOT_KERNEL_B_CONTINUE;
-    try_kernel_lversion[1] = kernelB_lversion;
-  } else {
-    try_kernel[0] = kernelB;
-    try_kernel_which[0] = BOOT_KERNEL_B_CONTINUE;
-    try_kernel_lversion[0] = kernelB_lversion;
-    try_kernel[1] = kernelA;
-    try_kernel_which[1] = BOOT_KERNEL_A_CONTINUE;
-    try_kernel_lversion[1] = kernelA_lversion;
-  }
-
-  /* TODO(gauravsh): Changes to boot_tries_remaining and boot_priority
-   * below should be propagated to partition table. This will be added
-   * once the firmware parition table parsing code is in. */
-  for (i = 0; i < 2; i++) {
-    if ((try_kernel[i]->boot_success_flag ||
-         try_kernel[i]->boot_tries_remaining) &&
-        (VERIFY_KERNEL_SUCCESS == VerifyKernel(firmware_key_blob,
-                                               try_kernel[i]->kernel_blob,
-                                               dev_mode))) {
-      if (try_kernel[i]->boot_tries_remaining > 0)
-        try_kernel[i]->boot_tries_remaining--;
-      if (stored_lversion > try_kernel_lversion[i])
-        continue;  /* Rollback: I am afraid I can't let you do that Dave. */
-      if (i == 0 && (stored_lversion < try_kernel_lversion[1])) {
-        /* The higher priority kernel is valid and bootable, See if we
-         * need to update the stored version for rollback prevention. */
-        if (VERIFY_KERNEL_SUCCESS == VerifyKernel(firmware_key_blob,
-                                                  try_kernel[1]->kernel_blob,
-                                                  dev_mode)) {
-          WriteStoredVersions(KERNEL_VERSIONS,
-                              (uint16_t) (min_lversion >> 16),
-                                (uint16_t) (min_lversion & 0xFFFF));
-          stored_lversion = min_lversion;  /* Update stored version as it's
-                                            * used later. */
-        }
-      }
-      kernel_to_boot = try_kernel_which[i];
-      break;  /* We found a valid kernel. */
-    }
-    try_kernel[i]->boot_priority = 0;
-    }  /* for loop. */
-
-  /* Lock Kernel TPM rollback indices from further writes.  In this design,
-   * this is tied to locking physical presence---so (software) physical
-   * presence cannot be asserted after this point.  This is a big side effect,
-   * so we want to make it clear in the function name.
-   * TODO(gauravsh): figure out better abstractions.
-   */
-  LockKernelVersionsByLockingPP();
-  return kernel_to_boot;
-}

vboot_firmware/linktest/main.c

@@ -31,7 +31,6 @@ int main(void)
   VerifyKernelHeader(0, 0, 0, 0, 0, 0);
   VerifyKernel(0, 0, 0);
   GetLogicalKernelVersion(0);
-  VerifyKernelDriver_f(0, 0, 0, 0);
 
   // load_kernel_fw.h
   LoadKernel(0);

vkernel/include/kernel_image.h

@@ -115,6 +115,4 @@ int AddKernelKeySignature(KernelImage* image, const char* firmware_key_file);
 int AddKernelSignature(KernelImage* image,
                        const char* kernel_sigining_key_file);
 
-void PrintKernelEntry(kernel_entry* entry);
-
 #endif  /* VBOOT_REFERENCE_KERNEL_IMAGE_H_ */

vkernel/kernel_image.c

@@ -599,15 +599,9 @@ int AddKernelSignature(KernelImage* image,
   return 1;
 }
 
-void PrintKernelEntry(kernel_entry* entry) {
-  debug("Boot Priority = %d\n", entry->boot_priority);
-  debug("Boot Tries Remaining = %d\n", entry->boot_tries_remaining);
-  debug("Boot Success Flag = %d\n", entry->boot_success_flag);
-}
-
-// Return the smallest integral multiple of [alignment] that is equal to or
-// greater than [val]. Used to determine the number of
-// pages/sectors/blocks/whatever needed to contain [val] items/bytes/etc.
+/* Return the smallest integral multiple of [alignment] that is equal to or
+ * greater than [val]. Used to determine the number of
+ * pages/sectors/blocks/whatever needed to contain [val] items/bytes/etc. */
 static uint64_t roundup(uint64_t val, uint64_t alignment) {
   uint64_t rem = val % alignment;
   if ( rem )
@@ -615,21 +609,21 @@ static uint64_t roundup(uint64_t val, uint64_t alignment) {
   return val;
 }
 
-// Match regexp /\b--\b/ to delimit the start of the kernel commandline. If we
-// don't find one, we'll use the whole thing.
+/* Match regexp /\b--\b/ to delimit the start of the kernel commandline. If we
+ * don't find one, we'll use the whole thing. */
 static unsigned int find_cmdline_start(char *input, unsigned int max_len) {
   int start = 0;
   int i;
   for(i = 0; i < max_len-1 && input[i]; i++) {
-    if (input[i] == '-' && input[i+1] == '-') { // found a "--"
-      if ((i == 0 || input[i-1] == ' ') && // nothing before it
-          (i+2 >= max_len || input[i+2] == ' ')) { // nothing after it
-        start = i+2;          // note: hope there's a trailing '\0'
+    if (input[i] == '-' && input[i+1] == '-') { /* found a "--" */
+      if ((i == 0 || input[i-1] == ' ') && /* nothing before it */
+          (i+2 >= max_len || input[i+2] == ' ')) { /* nothing after it */
+        start = i+2;          /* note: hope there's a trailing '\0' */
         break;
       }
     }
   }
-  while(input[start] == ' ')                    // skip leading spaces
+  while(input[start] == ' ')                    /* skip leading spaces */
     start++;
 
   return start;
@@ -659,7 +653,7 @@ uint8_t* GenerateKernelBlob(const char* kernel_file,
   uint32_t cmdline_addr;
   uint64_t i;
 
-  // Read the input files.
+  /* Read the input files. */
   kernel_buf = BufferFromFile(kernel_file, &kernel_size);
   if (!kernel_buf)
     goto done0;
@@ -667,13 +661,13 @@ uint8_t* GenerateKernelBlob(const char* kernel_file,
   config_buf = BufferFromFile(config_file, &config_size);
   if (!config_buf)
     goto done1;
-  if (config_size >= CROS_CONFIG_SIZE) { // need room for trailing '\0'
+  if (config_size >= CROS_CONFIG_SIZE) { /* need room for trailing '\0' */
     error("config file %s is too large (>= %d bytes)\n",
           config_file, CROS_CONFIG_SIZE);
     goto done1;
   }
 
-  // Replace any newlines with spaces in the config file.
+  /* Replace any newlines with spaces in the config file. */
   for (i=0; i < config_size; i++)
     if (config_buf[i] == '\n')
       config_buf[i] = ' ';
@@ -682,15 +676,15 @@ uint8_t* GenerateKernelBlob(const char* kernel_file,
   if (!bootloader_buf)
     goto done2;
 
-  // The first part of vmlinuz is a header, followed by a real-mode boot stub.
-  // We only want the 32-bit part.
+  /* The first part of vmlinuz is a header, followed by a real-mode boot stub.
+   * We only want the 32-bit part. */
   if (kernel_size) {
     lh = (struct linux_kernel_header *)kernel_buf;
     kernel32_start = (lh->setup_sects+1) << 9;
     kernel32_size = kernel_size - kernel32_start;
   }
 
-  // Allocate and zero the blob we need.
+  /* Allocate and zero the blob we need. */
   blob_size = roundup(kernel32_size, CROS_ALIGN) +
     CROS_CONFIG_SIZE +
     CROS_PARAMS_SIZE +
@@ -703,42 +697,43 @@ uint8_t* GenerateKernelBlob(const char* kernel_file,
   Memset(blob, 0, blob_size);
   now = 0;
 
-  // Copy the 32-bit kernel.
+  /* Copy the 32-bit kernel. */
   if (kernel32_size)
     Memcpy(blob + now, kernel_buf + kernel32_start, kernel32_size);
   now += roundup(now + kernel32_size, CROS_ALIGN);
 
-  // Find the load address of the commandline. We'll need it later.
+  /* Find the load address of the commandline. We'll need it later. */
   cmdline_addr = CROS_32BIT_ENTRY_ADDR + now
     + find_cmdline_start((char *)config_buf, config_size);
 
-  // Copy the config.
+  /* Copy the config. */
   if (config_size)
     Memcpy(blob + now, config_buf, config_size);
   now += CROS_CONFIG_SIZE;
 
-  // The zeropage data is next. Overlay the linux_kernel_header onto it, and
-  // tweak a few fields.
+  /* The zeropage data is next. Overlay the linux_kernel_header onto it, and
+   * tweak a few fields. */
   params = (struct linux_kernel_params *)(blob + now);
 
   if (kernel_size)
     Memcpy(&(params->setup_sects), &(lh->setup_sects),
            sizeof(*lh) - offsetof(struct linux_kernel_header, setup_sects));
   params->boot_flag = 0;
-  params->ramdisk_image = 0;             // we don't support initrd
+  params->ramdisk_image = 0;             /* we don't support initrd */
   params->ramdisk_size = 0;
   params->type_of_loader = 0xff;
   params->cmd_line_ptr = cmdline_addr;
   now += CROS_PARAMS_SIZE;
 
-  // Finally, append the bootloader. Remember where it will load in memory, too.
+  /* Finally, append the bootloader. Remember where it will load in memory, too.
+   */
   bootloader_mem_start = CROS_32BIT_ENTRY_ADDR + now;
   bootloader_mem_size = roundup(bootloader_size, CROS_ALIGN);
   if (bootloader_size)
     Memcpy(blob + now, bootloader_buf, bootloader_size);
   now += bootloader_mem_size;
 
-  // Pass back some info.
+  /* Pass back some info. */
   if (ret_blob_len)
     *ret_blob_len = blob_size;
   if (ret_bootloader_offset)
@@ -746,7 +741,7 @@ uint8_t* GenerateKernelBlob(const char* kernel_file,
   if (ret_bootloader_size)
     *ret_bootloader_size = bootloader_mem_size;
 
-  // Clean up and return the blob.
+  /* Clean up and return the blob. */
 done3:
   Free(bootloader_buf);
 done2: