Uses TPM return codes.
Rollback_index funcs now all return 0 if succcess, nonzero if error.
(Note: not fully implemented; Luigi, please clean this up in a
subsequent CL)
LoadKernel() checks return codes from TPM funcs.
LoadKernel() only looks at versions from TPM in normal boot mode.
Review URL: http://codereview.chromium.org/2735004
diff --git a/tests/rollback_index_mock.c b/tests/rollback_index_mock.c
index 9640e31..2e1e5ee 100644
--- a/tests/rollback_index_mock.c
+++ b/tests/rollback_index_mock.c
@@ -15,13 +15,14 @@
uint16_t g_kernel_key_version = 0;
uint16_t g_kernel_version = 0;
-void SetupTPM(void) {
+int SetupTPM(void) {
#ifndef NDEBUG
debug("Rollback Index Library Mock: TPM initialized.\n");
#endif
+ return 0;
}
-void GetStoredVersions(int type, uint16_t* key_version, uint16_t* version) {
+int GetStoredVersions(int type, uint16_t* key_version, uint16_t* version) {
switch (type) {
case FIRMWARE_VERSIONS:
*key_version = g_firmware_key_version;
@@ -32,6 +33,7 @@
*version = g_kernel_version;
break;
}
+ return 0;
}
int WriteStoredVersions(int type, uint16_t key_version, uint16_t version) {
@@ -48,16 +50,18 @@
#ifndef NDEBUG
debug("Rollback Index Library Mock: Stored Versions written.\n");
#endif
- return 1;
+ return 0;
}
-void LockFirmwareVersions() {
+int LockFirmwareVersions() {
#ifndef NDEBUG
debug("Rollback Index Library Mock: Firmware Versions Locked.\n");
#endif
+ return 0;
}
-void LockKernelVersionsByLockingPP() {
+int LockKernelVersionsByLockingPP() {
#ifndef NDEBUG
debug("Rollback Index Library Mock: Kernel Versions Locked.\n");
#endif
+ return 0;
}
diff --git a/vboot_firmware/include/rollback_index.h b/vboot_firmware/include/rollback_index.h
index 2096474..e0e9d27 100644
--- a/vboot_firmware/include/rollback_index.h
+++ b/vboot_firmware/include/rollback_index.h
@@ -27,11 +27,11 @@
#define KERNEL_VERSIONS_BACKUP_NV_INDEX 0x1004
#define KERNEL_BACKUP_IS_VALID_NV_INDEX 0x1005
-
-void SetupTPM(void);
-void GetStoredVersions(int type, uint16_t* key_version, uint16_t* version);
+/* All functions return 0 if successful, non-zero if error */
+int SetupTPM(void);
+int GetStoredVersions(int type, uint16_t* key_version, uint16_t* version);
int WriteStoredVersions(int type, uint16_t key_version, uint16_t version);
-void LockFirmwareVersions();
-void LockKernelVersionsByLockingPP();
+int LockFirmwareVersions();
+int LockKernelVersionsByLockingPP();
#endif /* VBOOT_REFERENCE_ROLLBACK_INDEX_H_ */
diff --git a/vboot_firmware/lib/load_kernel_fw.c b/vboot_firmware/lib/load_kernel_fw.c
index e79e245..cea6b77 100644
--- a/vboot_firmware/lib/load_kernel_fw.c
+++ b/vboot_firmware/lib/load_kernel_fw.c
@@ -63,12 +63,12 @@
}
-int AllocAndReadGptData(GptData *gptdata) {
- /* Allocates and reads GPT data from the drive. The sector_bytes and
- * drive_sectors fields should be filled on input. The primary and
- * secondary header and entries are filled on output.
- *
- * Returns 0 if successful, 1 if error. */
+/* Allocates and reads GPT data from the drive. The sector_bytes and
+ * drive_sectors fields should be filled on input. The primary and
+ * secondary header and entries are filled on output.
+ *
+ * Returns 0 if successful, 1 if error. */
+int AllocAndReadGptData(GptData* gptdata) {
uint64_t entries_sectors = GPT_ENTRIES_SIZE / gptdata->sector_bytes;
@@ -100,42 +100,55 @@
return 0;
}
-void WriteAndFreeGptData(GptData *gptdata) {
- /* Writes any changes for the GPT data back to the drive, then frees the
- * buffers. */
+
+/* Writes any changes for the GPT data back to the drive, then frees
+ * the buffers.
+ *
+ * Returns 0 if successful, 1 if error. */
+int WriteAndFreeGptData(GptData* gptdata) {
uint64_t entries_sectors = GPT_ENTRIES_SIZE / gptdata->sector_bytes;
if (gptdata->primary_header) {
- if (gptdata->modified & GPT_MODIFIED_HEADER1)
- BootDeviceWriteLBA(1, 1, gptdata->primary_header);
+ if (gptdata->modified & GPT_MODIFIED_HEADER1) {
+ if (0 != BootDeviceWriteLBA(1, 1, gptdata->primary_header))
+ return 1;
+ }
Free(gptdata->primary_header);
}
if (gptdata->primary_entries) {
- if (gptdata->modified & GPT_MODIFIED_ENTRIES1)
- BootDeviceWriteLBA(2, entries_sectors, gptdata->primary_entries);
+ if (gptdata->modified & GPT_MODIFIED_ENTRIES1) {
+ if (0 != BootDeviceWriteLBA(2, entries_sectors,
+ gptdata->primary_entries))
+ return 1;
+ }
Free(gptdata->primary_entries);
}
if (gptdata->secondary_entries) {
- if (gptdata->modified & GPT_MODIFIED_ENTRIES2)
- BootDeviceWriteLBA(gptdata->drive_sectors - entries_sectors - 1,
- entries_sectors, gptdata->secondary_entries);
+ if (gptdata->modified & GPT_MODIFIED_ENTRIES2) {
+ if (0 != BootDeviceWriteLBA(gptdata->drive_sectors - entries_sectors - 1,
+ entries_sectors, gptdata->secondary_entries))
+ return 1;
+ }
Free(gptdata->secondary_entries);
}
if (gptdata->secondary_header) {
- if (gptdata->modified & GPT_MODIFIED_HEADER2)
- BootDeviceWriteLBA(gptdata->drive_sectors - entries_sectors - 1,
- 1, gptdata->secondary_header);
- BootDeviceWriteLBA(gptdata->drive_sectors - 1, 1,
- gptdata->secondary_header);
+ if (gptdata->modified & GPT_MODIFIED_HEADER2) {
+ if (0 != BootDeviceWriteLBA(gptdata->drive_sectors - 1, 1,
+ gptdata->secondary_header))
+ return 1;
+ }
Free(gptdata->secondary_header);
}
- /* TODO: What to do with return codes from the writes? */
+
+ /* Success */
+ return 0;
}
+
#define KBUF_SIZE 65536 /* Bytes to read at start of kernel partition */
int LoadKernel(LoadKernelParams* params) {
@@ -157,15 +170,14 @@
params->bootloader_address = 0;
params->bootloader_size = 0;
- /* Read current kernel key index from TPM. Assumes TPM is already
- * initialized. */
- /* TODO: Is that a safe assumption? Normally, SetupTPM() would be called
- * when the RW firmware is verified. Is it harmful to call SetupTPM()
- * again if it's already initialized? It'd be easier if we could just do
- * that. */
- GetStoredVersions(KERNEL_VERSIONS,
- &tpm_kernel_key_version,
- &tpm_kernel_version);
+ if (BOOT_MODE_NORMAL == params->boot_mode) {
+ /* Read current kernel key index from TPM. Assumes TPM is already
+ * initialized. */
+ if (0 != GetStoredVersions(KERNEL_VERSIONS,
+ &tpm_kernel_key_version,
+ &tpm_kernel_version))
+ return LOAD_KERNEL_RECOVERY;
+ }
do {
/* Read GPT data */
@@ -290,9 +302,16 @@
params->bootloader_address = kim->bootloader_offset;
params->bootloader_size = kim->bootloader_size;
- /* 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 we're in developer or recovery mode, there's no rollback
+ * protection, so we can stop at the first valid kernel. */
+ if (BOOT_MODE_NORMAL != params->boot_mode)
+ 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 (kim->kernel_key_version == tpm_kernel_key_version &&
kim->kernel_version == tpm_kernel_version)
break;
@@ -306,19 +325,22 @@
if (kim)
Free(kim);
- // Write and free GPT data
+ /* Write and free GPT data */
WriteAndFreeGptData(&gpt);
- // Handle finding a good partition
+ /* Handle finding a good partition */
if (good_partition >= 0) {
- /* See if we need to update the TPM */
- if ((lowest_kernel_key_version > tpm_kernel_key_version) ||
- (lowest_kernel_key_version == tpm_kernel_key_version &&
- lowest_kernel_version > tpm_kernel_version)) {
- WriteStoredVersions(KERNEL_VERSIONS,
- lowest_kernel_key_version,
- lowest_kernel_version);
+ if (BOOT_MODE_NORMAL == params->boot_mode) {
+ /* See if we need to update the TPM, for normal boot mode only. */
+ if ((lowest_kernel_key_version > tpm_kernel_key_version) ||
+ (lowest_kernel_key_version == tpm_kernel_key_version &&
+ lowest_kernel_version > tpm_kernel_version)) {
+ if (0 != WriteStoredVersions(KERNEL_VERSIONS,
+ lowest_kernel_key_version,
+ lowest_kernel_version))
+ return LOAD_KERNEL_RECOVERY;
+ }
}
if (BOOT_MODE_RECOVERY != params->boot_mode) {
@@ -330,19 +352,17 @@
*
* If we're already in recovery mode, we need to leave PP unlocked,
* so don't lock the kernel versions. */
- LockKernelVersionsByLockingPP();
+ if (0 != LockKernelVersionsByLockingPP())
+ return LOAD_KERNEL_RECOVERY;
}
/* Success! */
return LOAD_KERNEL_SUCCESS;
}
- // Handle error cases
+ /* Handle error cases */
if (found_partition)
return LOAD_KERNEL_INVALID;
else
return LOAD_KERNEL_NOT_FOUND;
- /* TODO: no error code for "internal error", but what would the firmware do
- * with that anyway? So in the do-while(0) code above, the firmware just
- * does 'break' to indicate an internal error... */
}
diff --git a/vboot_firmware/lib/rollback_index.c b/vboot_firmware/lib/rollback_index.c
index 9ce523b..fe0caff 100644
--- a/vboot_firmware/lib/rollback_index.c
+++ b/vboot_firmware/lib/rollback_index.c
@@ -172,7 +172,7 @@
}
-void SetupTPM(void) {
+int SetupTPM(void) {
uint8_t disable;
uint8_t deactivated;
TlclLibinit();
@@ -189,13 +189,13 @@
/* Check that the TPM is enabled and activated. */
if(TlclGetFlags(&disable, &deactivated) != TPM_SUCCESS) {
debug("failed to get TPM flags");
- EnterRecovery(1);
+ return 1;
}
if (disable || deactivated) {
TlclSetEnable();
if (TlclSetDeactivated(0) != TPM_SUCCESS) {
debug("failed to activate TPM");
- EnterRecovery(1);
+ return 1;
}
}
/* We expect this to fail the first time we run on a device, indicating that
@@ -205,12 +205,22 @@
if (!InitializeSpaces()) {
/* If InitializeSpaces() fails (possibly because it had been executed
* already), something is wrong. */
- EnterRecovery(1);
+ return 1;
}
}
+
+ return 0;
}
-void GetStoredVersions(int type, uint16_t* key_version, uint16_t* version) {
+int GetStoredVersions(int type, uint16_t* key_version, uint16_t* version) {
+
+ /* TODO: should verify that SetupTPM() has been called. Note that
+ * SetupTPM() does hardware setup AND sets global variables. When we
+ * get down into kernel verification, the hardware setup persists, but
+ * we don't have access to the global variables. So I guess we DO need
+ * to call SetupTPM() there, and have it be smart enough not to redo the
+ * hardware init, but it still needs to re-read the flags... */
+
switch (type) {
case FIRMWARE_VERSIONS:
*key_version = g_firmware_key_version;
@@ -221,37 +231,40 @@
*version = g_kernel_version;
break;
}
+
+ return 0;
}
int WriteStoredVersions(int type, uint16_t key_version, uint16_t version) {
uint32_t combined_version = (key_version << 16) & version;
switch (type) {
case FIRMWARE_VERSIONS:
- return (TPM_SUCCESS == TlclWrite(FIRMWARE_VERSIONS_NV_INDEX,
+ return (TPM_SUCCESS != TlclWrite(FIRMWARE_VERSIONS_NV_INDEX,
(uint8_t*) &combined_version,
sizeof(uint32_t)));
- break;
+
case KERNEL_VERSIONS:
- return (TPM_SUCCESS == TlclWrite(KERNEL_VERSIONS_NV_INDEX,
+ return (TPM_SUCCESS != TlclWrite(KERNEL_VERSIONS_NV_INDEX,
(uint8_t*) &combined_version,
sizeof(uint32_t)));
- break;
}
/* TODO(nelson): ForceClear and reboot if unowned. */
+ return 1;
+}
+
+int LockFirmwareVersions() {
+ if (TlclSetGlobalLock() != TPM_SUCCESS) {
+ debug("failed to set global lock");
+ return 1;
+ }
return 0;
}
-void LockFirmwareVersions() {
- if (TlclSetGlobalLock() != TPM_SUCCESS) {
- debug("failed to set global lock");
- EnterRecovery(1);
- }
-}
-
-void LockKernelVersionsByLockingPP() {
+int LockKernelVersionsByLockingPP() {
if (TlclLockPhysicalPresence() != TPM_SUCCESS) {
debug("failed to turn off PP");
- EnterRecovery(1);
+ return 1;
}
+ return 0;
}