Ignore TPM return codes in recovery mode

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 106 matching lines @@
   uint64_t part_start, part_size;
   uint64_t blba = params->bytes_per_lba;
   uint64_t kbuf_sectors = KBUF_SIZE / blba;
   uint8_t* kbuf = NULL;
   int found_partitions = 0;
   int good_partition = -1;
   uint16_t tpm_key_version = 0;
   uint16_t tpm_kernel_version = 0;
   uint64_t lowest_key_version = 0xFFFF;
   uint64_t lowest_kernel_version = 0xFFFF;
-  int is_dev = ((BOOT_FLAG_DEVELOPER & params->boot_flags) &&
-                !(BOOT_FLAG_RECOVERY & params->boot_flags));
-  int is_normal = (!(BOOT_FLAG_DEVELOPER & params->boot_flags) &&
-                   !(BOOT_FLAG_RECOVERY & params->boot_flags));
+  int is_dev = (BOOT_FLAG_DEVELOPER & params->boot_flags);
+  int is_rec = (BOOT_FLAG_RECOVERY & params->boot_flags);
+  int is_normal = (!is_dev && !is_rec);
 
   /* Clear output params in case we fail */
   params->partition_number = 0;
   params->bootloader_address = 0;
   params->bootloader_size = 0;
 
   /* Let the TPM know if we're in recovery mode */
-  if (BOOT_FLAG_RECOVERY & params->boot_flags) {
-    if (0 != RollbackKernelRecovery(BOOT_FLAG_DEVELOPER & params->boot_flags
-                                    ? 1 : 0)) {
+  if (is_rec) {
+    if (0 != RollbackKernelRecovery(is_dev ? 1 : 0)) {

[Luigi Semenzato, 2010/06/24 20:18:43]

If our convention for booleans is 0 for false, and anything else for true, you
could just pass is_dev here.  That seems to be the convention based on your use
below.

       VBDEBUG(("Error setting up TPM for recovery kernel\n"));
-      return LOAD_KERNEL_RECOVERY;
+      /* Ignore return code, since we need to boot recovery mode to
+       * fix the TPM. */
     }
   }
 
   if (is_normal) {
     /* Read current kernel key index from TPM.  Assumes TPM is already
      * initialized. */
     if (0 != RollbackKernelRead(&tpm_key_version, &tpm_kernel_version)) {
       VBDEBUG(("Unable to get kernel versions from TPM\n"));
       return LOAD_KERNEL_RECOVERY;
     }
-  } else if (is_dev) {
+  } else if (is_dev && !is_rec) {
     /* In developer mode, we ignore the kernel subkey, and just use
      * the SHA-512 hash to verify the key block. */
     kernel_subkey = NULL;
   }
 
   do {
     /* Read GPT data */
     gpt.sector_bytes = (uint32_t)blba;
     gpt.drive_sectors = params->ending_lba + 1;
     if (0 != AllocAndReadGptData(&gpt)) {
@@ skipping 34 matching lines @@
 
       /* Verify the key block */
       key_block = (VbKeyBlockHeader*)kbuf;
       if ((0 != KeyBlockVerify(key_block, KBUF_SIZE, kernel_subkey))) {
         VBDEBUG(("Verifying key block failed.\n"));
         continue;
       }
 
       /* Check the key block flags against the current boot mode */
       if (!(key_block->key_block_flags &&
-            ((BOOT_FLAG_DEVELOPER & params->boot_flags) ?
-             KEY_BLOCK_FLAG_DEVELOPER_1 : KEY_BLOCK_FLAG_DEVELOPER_0))) {
+            (is_dev ? KEY_BLOCK_FLAG_DEVELOPER_1 :
+             KEY_BLOCK_FLAG_DEVELOPER_0))) {
         VBDEBUG(("Developer flag mismatch.\n"));
         continue;
       }
       if (!(key_block->key_block_flags &&
-            ((BOOT_FLAG_RECOVERY & params->boot_flags) ?
-             KEY_BLOCK_FLAG_RECOVERY_1 : KEY_BLOCK_FLAG_RECOVERY_0))) {
+            (is_rec ? KEY_BLOCK_FLAG_RECOVERY_1 :
+             KEY_BLOCK_FLAG_RECOVERY_0))) {
         VBDEBUG(("Recovery flag mismatch.\n"));
         continue;
       }
 
       /* 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_key_version) {
         VBDEBUG(("Key version too old.\n"));
@@ skipping 141 matching lines @@
           (lowest_key_version == tpm_key_version &&
            lowest_kernel_version > tpm_kernel_version)) {
         if (0 != RollbackKernelWrite((uint16_t)lowest_key_version,
                                      (uint16_t)lowest_kernel_version)) {
           VBDEBUG(("Error writing kernel versions to TPM.\n"));
           return LOAD_KERNEL_RECOVERY;
         }
       }
     }
 
-    /* Lock the kernel versions, since we're about to boot the kernel */
+    /* Lock the kernel versions */

[Luigi Semenzato, 2010/06/24 20:18:43]

I think that here we may want to invert the conditionals.  We don't want to lock
PP in recovery mode.

Oh I see.  I already take care of that in RollbackKernelLock.  So you don't
really have to check for is_rec, because in that case the Lock call is a noop,
which is always successful.

Either way is fine.

     if (0 != RollbackKernelLock()) {
       VBDEBUG(("Error locking kernel versions.\n"));
-      return LOAD_KERNEL_RECOVERY;
+      /* Don't reboot to recovery mode if we're already there */
+      if (!is_rec)
+        return LOAD_KERNEL_RECOVERY;
     }
 
     /* Success! */
     return LOAD_KERNEL_SUCCESS;
   }
 
   // Handle error cases
   if (found_partitions)
     return LOAD_KERNEL_INVALID;
   else
     return LOAD_KERNEL_NOT_FOUND;
 }