Fix LoadKernel() failing to update GPT flags.

firmware/lib/vboot_kernel.c

 /* Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  *
  * Functions for loading a kernel from disk.
  * (Firmware portion)
  */
 
 #include "vboot_kernel.h"
 
@@ skipping 181 matching lines @@
 
     /* Allocate kernel header buffers */
     kbuf = (uint8_t*)Malloc(KBUF_SIZE);
     if (!kbuf)
       break;
 
     /* Loop over candidate kernel partitions */
     while (GPT_SUCCESS == GptNextKernelEntry(&gpt, &part_start, &part_size)) {
       VbKeyBlockHeader* key_block;
       VbKernelPreambleHeader* preamble;
-      RSAPublicKey* data_key;
+      RSAPublicKey* data_key = NULL;
       uint64_t key_version;
       uint64_t combined_version;
       uint64_t body_offset;
       uint64_t body_offset_sectors;
       uint64_t body_sectors;
 
       VBDEBUG(("Found kernel entry at %" PRIu64 " size %" PRIu64 "\n",
               part_start, part_size));
 
       /* Found at least one kernel partition. */
       found_partitions++;
 
       /* Read the first part of the kernel partition  */
       if (part_size < kbuf_sectors)
-        continue;
+        goto bad_kernel;
       if (0 != BootDeviceReadLBA(part_start, kbuf_sectors, kbuf))
-        continue;
+        goto bad_kernel;
 
       /* Verify the key block.  In developer mode, we ignore the key
        * and use only the SHA-512 hash to verify the key block. */
       key_block = (VbKeyBlockHeader*)kbuf;
       if ((0 != KeyBlockVerify(key_block, KBUF_SIZE, kernel_subkey,
                                is_dev && !is_rec))) {
         VBDEBUG(("Verifying key block failed.\n"));
-        continue;
+        goto bad_kernel;
       }
 
       /* Check the key block flags against the current boot mode in normal
        * and recovery modes (not in developer mode booting from SSD). */
       if (is_rec || is_normal) {
         if (!(key_block->key_block_flags &
               (is_dev ? KEY_BLOCK_FLAG_DEVELOPER_1 :
                KEY_BLOCK_FLAG_DEVELOPER_0))) {
           VBDEBUG(("Developer flag mismatch.\n"));
-          continue;
+          goto bad_kernel;
         }
         if (!(key_block->key_block_flags &
               (is_rec ? KEY_BLOCK_FLAG_RECOVERY_1 :
                KEY_BLOCK_FLAG_RECOVERY_0))) {
           VBDEBUG(("Recovery flag mismatch.\n"));
-          continue;
+          goto bad_kernel;
         }
       }
 
       /* Check for rollback of key version.  Note this is implicitly
        * skipped in recovery and developer modes because those set
        * key_version=0 above. */
       key_version = key_block->data_key.key_version;
       if (key_version < (tpm_version >> 16)) {
         VBDEBUG(("Key version too old.\n"));
-        continue;
+        goto bad_kernel;
       }
 
       /* Get the key for preamble/data verification from the key block */
       data_key = PublicKeyToRSA(&key_block->data_key);
       if (!data_key)
-        continue;
+        goto bad_kernel;
 
       /* Verify the preamble, which follows the key block */
       preamble = (VbKernelPreambleHeader*)(kbuf + key_block->key_block_size);
       if ((0 != VerifyKernelPreamble(preamble,
                                      KBUF_SIZE - key_block->key_block_size,
                                      data_key))) {
         VBDEBUG(("Preamble verification failed.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Check for rollback of kernel version.  Note this is implicitly
        * skipped in recovery and developer modes because rollback_index
        * sets those to 0 in those modes. */
       combined_version = ((key_version << 16) |
                           (preamble->kernel_version & 0xFFFF));
       if (combined_version < tpm_version) {
         VBDEBUG(("Kernel version too low.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       VBDEBUG(("Kernel preamble is good.\n"));
 
       /* Check for lowest version from a valid header. */
       if (lowest_version > combined_version)
         lowest_version = combined_version;
 
       /* If we already have a good kernel, no need to read another
        * one; we only needed to look at the versions to check for
        * rollback. */
       if (-1 != good_partition)
-        continue;
+        goto bad_kernel;

[Randall Spangler, 2010/08/20 21:20:47]

Heh, note that we'd previously missed freeing the data key here...

 
       /* Verify body load address matches what we expect */
       if ((preamble->body_load_address != (size_t)params->kernel_buffer) &&
           !(params->boot_flags & BOOT_FLAG_SKIP_ADDR_CHECK)) {
         VBDEBUG(("Wrong body load address.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Verify kernel body starts at a multiple of the sector size. */
       body_offset = key_block->key_block_size + preamble->preamble_size;
       if (0 != body_offset % blba) {
         VBDEBUG(("Kernel body not at multiple of sector size.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
       body_offset_sectors = body_offset / blba;
 
       /* Verify kernel body fits in the buffer */
       body_sectors = (preamble->body_signature.data_size + blba - 1) / blba;
       if (body_sectors * blba > params->kernel_buffer_size) {
         VBDEBUG(("Kernel body doesn't fit in memory.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Verify kernel body fits in the partition */
       if (body_offset_sectors + body_sectors > part_size) {
         VBDEBUG(("Kernel body doesn't fit in partition.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Read the kernel data */
       if (0 != BootDeviceReadLBA(part_start + body_offset_sectors,
                                  body_sectors,
                                  params->kernel_buffer)) {
         VBDEBUG(("Unable to read kernel data.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Verify kernel data */
       if (0 != VerifyData((const uint8_t*)params->kernel_buffer,
                           params->kernel_buffer_size,
                           &preamble->body_signature, data_key)) {
         VBDEBUG(("Kernel data verification failed.\n"));
-        RSAPublicKeyFree(data_key);
-        continue;
+        goto bad_kernel;
       }
 
       /* Done with the kernel signing key, so can free it now */
       RSAPublicKeyFree(data_key);
+      data_key = NULL;
 
       /* If we're still here, the kernel is valid. */
       /* Save the first good partition we find; that's the one we'll boot */
       VBDEBUG(("Partiton is good.\n"));
       /* TODO: GPT partitions start at 1, but cgptlib starts them at 0.
        * Adjust here, until cgptlib is fixed. */
       good_partition = gpt.current_kernel + 1;
       params->partition_number = gpt.current_kernel + 1;
       GetCurrentKernelUniqueGuid(&gpt, &params->partition_guid);
       /* TODO: GetCurrentKernelUniqueGuid() should take a destination size, or
        * the dest should be a struct, so we know it's big enough. */
       params->bootloader_address = preamble->bootloader_address;
       params->bootloader_size = preamble->bootloader_size;
+
+      /* Update GPT to note this is the kernel we're trying */
+      GptUpdateKernelEntry(&gpt, GPT_UPDATE_ENTRY_TRY);
+
       /* If we're in developer or recovery mode, there's no rollback
        * protection, so we can stop at the first valid kernel. */
       if (!is_normal) {
         VBDEBUG(("Boot_flags = !is_normal\n"));
         break;
       }
 
       /* Otherwise, we're in normal boot mode, so we do care about the
        * key index in the TPM.  If the good partition's key version is
        * the same as the tpm, then the TPM doesn't need updating; we
        * can stop now.  Otherwise, we'll check all the other headers
        * to see if they contain a newer key. */
       if (combined_version == tpm_version) {
         VBDEBUG(("Same kernel version\n"));
         break;
       }
+
+      /* Continue, so that we skip the error handling code below */
+      continue;
+
+   bad_kernel:
+      /* Handle errors parsing this kernel */
+      if (NULL != data_key)

[gauravsh, 2010/08/20 21:29:01]

this null check is not needed. All the *Free() functions follow the free()
semantics. Null pointer input does nothing.

+        RSAPublicKeyFree(data_key);
+
+      VBDEBUG(("Marking kernel as invalid.\n"));
+      GptUpdateKernelEntry(&gpt, GPT_UPDATE_ENTRY_BAD);
+
+
     } /* while(GptNextKernelEntry) */
   } while(0);
 
   /* Free kernel buffer */
   if (kbuf)
     Free(kbuf);
 
   /* Write and free GPT data */
   WriteAndFreeGptData(&gpt);
 
@@ skipping 33 matching lines @@
     /* Success! */
     return LOAD_KERNEL_SUCCESS;
   }
 
   // Handle error cases
   if (found_partitions)
     return LOAD_KERNEL_INVALID;
   else
     return LOAD_KERNEL_NOT_FOUND;
 }