diff options
author | Joel Kitching <kitching@google.com> | 2019-10-01 16:02:44 +0800 |
---|---|---|
committer | Hung-Te Lin <hungte@chromium.org> | 2019-11-07 02:17:39 +0000 |
commit | 41be83a361de477138f41132a746553b3a518768 (patch) | |
tree | aa76a95eb3758b1abb97a7a78ec8c0971372b7cf | |
parent | f2078c671da786e4b2db6a16aa5c83ba93373012 (diff) | |
download | vboot-41be83a361de477138f41132a746553b3a518768.tar.gz |
vboot: remove VbSetRecoveryRequest
Remove VbSetRecoveryRequest and use vb2api_fail instead.
When failure is encountered in kernel verification, it's very
possible that there is a bug in updated RW firmware. The other
firmware slot should always be attempted before falling back to
recovery mode. Call vb2api_fail to invoke this behaviour, rather
than setting the recovery request directly with
VbSetRecoveryRequest.
BUG=b:124141368, chromium:1007999
TEST=make clean && make runtests
BRANCH=none
Change-Id: I69c457f37d1f58c1eef33dec436fb77b2a77030f
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1833364
Tested-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
(cherry picked from commit f06f7551e16bb5e44b3b1f2fd5788ea86825cd7e)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1902828
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Auto-Submit: Hung-Te Lin <hungte@chromium.org>
-rw-r--r-- | firmware/lib/vboot_api_kernel.c | 108 | ||||
-rw-r--r-- | tests/vboot_api_kernel_tests.c | 7 |
2 files changed, 63 insertions, 52 deletions
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index 853fd82c..906341dd 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -40,16 +40,6 @@ struct LoadKernelParams *VbApiKernelGetParams(void) #endif -/** - * Set recovery request (called from vboot_api_kernel.c functions only) - */ -static void VbSetRecoveryRequest(struct vb2_context *ctx, - uint32_t recovery_request) -{ - VB2_DEBUG("VbSetRecoveryRequest(%d)\n", (int)recovery_request); - vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, recovery_request); -} - void vb2_nv_commit(struct vb2_context *ctx) { /* Exit if nothing has changed */ @@ -67,7 +57,7 @@ uint32_t vb2_get_fwmp_flags(void) vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t get_info_flags) { - vb2_error_t retval = VB2_ERROR_UNKNOWN; + vb2_error_t rv; VbDiskInfo* disk_info = NULL; uint32_t disk_count = 0; uint32_t i; @@ -79,13 +69,13 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t get_info_flags) lkp.disk_handle = NULL; /* Find disks */ - if (VB2_SUCCESS != VbExDiskGetInfo(&disk_info, &disk_count, - get_info_flags)) + rv = VbExDiskGetInfo(&disk_info, &disk_count, get_info_flags); + if (VB2_SUCCESS != rv) disk_count = 0; VB2_DEBUG("VbTryLoadKernel() found %d disks\n", (int)disk_count); if (0 == disk_count) { - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_NO_DISK); + vb2api_fail(ctx, VB2_RECOVERY_RW_NO_DISK, rv); return VBERROR_NO_DISK_FOUND; } @@ -119,9 +109,9 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t get_info_flags) ?: lkp.gpt_lba_count; lkp.boot_flags |= disk_info[i].flags & VB_DISK_FLAG_EXTERNAL_GPT ? BOOT_FLAG_EXTERNAL_GPT : 0; - retval = LoadKernel(ctx, &lkp); + rv = LoadKernel(ctx, &lkp); - VB2_DEBUG("VbTryLoadKernel() LoadKernel() = %d\n", retval); + VB2_DEBUG("VbTryLoadKernel() LoadKernel() = %d\n", rv); /* * Stop now if we found a kernel. @@ -130,13 +120,13 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t get_info_flags) * get, instead of just returning the value from the last disk * attempted. */ - if (VB2_SUCCESS == retval) + if (VB2_SUCCESS == rv) break; } /* If we didn't find any good kernels, don't return a disk handle. */ - if (VB2_SUCCESS != retval) { - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_NO_KERNEL); + if (VB2_SUCCESS != rv) { + vb2api_fail(ctx, VB2_RECOVERY_RW_NO_KERNEL, rv); lkp.disk_handle = NULL; } @@ -146,7 +136,7 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t get_info_flags) * Pass through return code. Recovery reason (if any) has already been * set by LoadKernel(). */ - return retval; + return rv; } /** @@ -237,11 +227,14 @@ vb2_error_t VbBootNormal(struct vb2_context *ctx) shared->kernel_version_tpm = max_rollforward; } - if ((shared->kernel_version_tpm > shared->kernel_version_tpm_start) && - RollbackKernelWrite(shared->kernel_version_tpm)) { - VB2_DEBUG("Error writing kernel versions to TPM.\n"); - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_TPM_W_ERROR); - return VBERROR_TPM_WRITE_KERNEL; + if (shared->kernel_version_tpm > shared->kernel_version_tpm_start) { + uint32_t tpm_rv = + RollbackKernelWrite(shared->kernel_version_tpm); + if (tpm_rv) { + VB2_DEBUG("Error writing kernel versions to TPM.\n"); + vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_W_ERROR, tpm_rv); + return VBERROR_TPM_WRITE_KERNEL; + } } return rv; @@ -251,9 +244,13 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx, VbSharedDataHeader *shared, VbSelectAndLoadKernelParams *kparams) { - if (VB2_SUCCESS != vb2_init_context(ctx)) { + vb2_error_t rv; + uint32_t tpm_rv; + + rv = vb2_init_context(ctx); + if (VB2_SUCCESS != rv) { VB2_DEBUG("Can't init vb2_context\n"); - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_SHARED_DATA); + vb2api_fail(ctx, VB2_RECOVERY_RW_SHARED_DATA, rv); return VBERROR_INIT_SHARED_DATA; } @@ -321,10 +318,11 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx, memset(kparams->partition_guid, 0, sizeof(kparams->partition_guid)); /* Read kernel version from the TPM. Ignore errors in recovery mode. */ - if (RollbackKernelRead(&shared->kernel_version_tpm)) { + tpm_rv = RollbackKernelRead(&shared->kernel_version_tpm); + if (tpm_rv) { VB2_DEBUG("Unable to get kernel versions from TPM\n"); if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_TPM_R_ERROR); + vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_R_ERROR, tpm_rv); return VBERROR_TPM_READ_KERNEL; } } @@ -334,10 +332,14 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx, /* Read FWMP. Ignore errors in recovery mode. */ if (gbb->flags & VB2_GBB_FLAG_DISABLE_FWMP) { memset(&fwmp, 0, sizeof(fwmp)); - } else if (RollbackFwmpRead(&fwmp)) { + return VB2_SUCCESS; + } + + tpm_rv = RollbackFwmpRead(&fwmp); + if (tpm_rv) { VB2_DEBUG("Unable to get FWMP from TPM\n"); if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_TPM_R_ERROR); + vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_R_ERROR, tpm_rv); return VBERROR_TPM_READ_FWMP; } } @@ -362,11 +364,13 @@ static vb2_error_t vb2_kernel_phase4(struct vb2_context *ctx, sizeof(kparams->partition_guid)); /* Lock the kernel versions if not in recovery mode */ - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && - RollbackKernelLock(sd->recovery_reason)) { - VB2_DEBUG("Error locking kernel versions.\n"); - VbSetRecoveryRequest(ctx, VB2_RECOVERY_RW_TPM_L_ERROR); - return VBERROR_TPM_LOCK_KERNEL; + if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { + uint32_t tpm_rv = RollbackKernelLock(sd->recovery_reason); + if (tpm_rv) { + VB2_DEBUG("Error locking kernel versions.\n"); + vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_L_ERROR, tpm_rv); + return VBERROR_TPM_LOCK_KERNEL; + } } return VB2_SUCCESS; @@ -388,8 +392,8 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, VbSharedDataHeader *shared, VbSelectAndLoadKernelParams *kparams) { - vb2_error_t retval = vb2_kernel_setup(ctx, shared, kparams); - if (retval) + vb2_error_t rv = vb2_kernel_setup(ctx, shared, kparams); + if (rv) goto VbSelectAndLoadKernel_exit; VB2_DEBUG("GBB flags are %#x\n", vb2_get_gbb(ctx)->flags); @@ -399,8 +403,8 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, * it's just a single non-interactive WAIT screen. */ if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - retval = ec_sync_all(ctx); - if (retval) + rv = ec_sync_all(ctx); + if (rv) goto VbSelectAndLoadKernel_exit; } @@ -408,9 +412,9 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) { /* Recovery boot. This has UI. */ if (kparams->inflags & VB_SALK_INFLAGS_ENABLE_DETACHABLE_UI) - retval = VbBootRecoveryMenu(ctx); + rv = VbBootRecoveryMenu(ctx); else - retval = VbBootRecovery(ctx); + rv = VbBootRecovery(ctx); VbExEcEnteringMode(0, VB_EC_RECOVERY); } else if (DIAGNOSTIC_UI && vb2_nv_get(ctx, VB2_NV_DIAG_REQUEST)) { vb2_nv_set(ctx, VB2_NV_DIAG_REQUEST, 0); @@ -421,38 +425,38 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, * needed. This mode is also 1-shot so it's placed * before developer mode. */ - retval = VbBootDiagnostic(ctx); + rv = VbBootDiagnostic(ctx); /* * The diagnostic menu should either boot a rom, or * return either of reboot or shutdown. The following * check is a safety precaution. */ - if (!retval) - retval = VBERROR_REBOOT_REQUIRED; + if (!rv) + rv = VBERROR_REBOOT_REQUIRED; } else if (ctx->flags & VB2_CONTEXT_DEVELOPER_MODE) { if (kparams->inflags & VB_SALK_INFLAGS_VENDOR_DATA_SETTABLE) ctx->flags |= VB2_CONTEXT_VENDOR_DATA_SETTABLE; /* Developer boot. This has UI. */ if (kparams->inflags & VB_SALK_INFLAGS_ENABLE_DETACHABLE_UI) - retval = VbBootDeveloperMenu(ctx); + rv = VbBootDeveloperMenu(ctx); else - retval = VbBootDeveloper(ctx); + rv = VbBootDeveloper(ctx); VbExEcEnteringMode(0, VB_EC_DEVELOPER); } else { /* Normal boot */ - retval = VbBootNormal(ctx); + rv = VbBootNormal(ctx); VbExEcEnteringMode(0, VB_EC_NORMAL); } VbSelectAndLoadKernel_exit: - if (VB2_SUCCESS == retval) - retval = vb2_kernel_phase4(ctx, kparams); + if (VB2_SUCCESS == rv) + rv = vb2_kernel_phase4(ctx, kparams); vb2_kernel_cleanup(ctx); /* Pass through return value from boot path */ - VB2_DEBUG("Returning %d\n", (int)retval); - return retval; + VB2_DEBUG("Returning %d\n", (int)rv); + return rv; } diff --git a/tests/vboot_api_kernel_tests.c b/tests/vboot_api_kernel_tests.c index a5de12ba..69128730 100644 --- a/tests/vboot_api_kernel_tests.c +++ b/tests/vboot_api_kernel_tests.c @@ -6,6 +6,7 @@ */ #include "2common.h" +#include "2misc.h" #include "2nvstorage.h" #include "2sysincludes.h" #include "load_kernel_fw.h" @@ -220,6 +221,7 @@ static const char *got_find_disk; static const char *got_load_disk; static uint32_t got_return_val; static uint32_t got_external_mismatch; +static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE]; static struct vb2_context ctx; /** @@ -228,6 +230,9 @@ static struct vb2_context ctx; static void ResetMocks(int i) { memset(&ctx, 0, sizeof(ctx)); + ctx.workbuf = workbuf; + ctx.workbuf_size = sizeof(workbuf); + vb2_init_context(&ctx); memset(VbApiKernelGetParams(), 0, sizeof(LoadKernelParams)); @@ -328,6 +333,8 @@ void vb2_nv_set(struct vb2_context *c, enum vb2_nv_param param, uint32_t value) { + if (param != VB2_NV_RECOVERY_REQUEST) + return; VB2_DEBUG("%s(): got_recovery_request_val = %d (0x%x)\n", __FUNCTION__, value, value); got_recovery_request_val = value; |