diff options
-rw-r--r-- | firmware/lib/include/vboot_kernel.h | 13 | ||||
-rw-r--r-- | firmware/lib/vboot_api_kernel.c | 86 | ||||
-rw-r--r-- | firmware/lib/vboot_display.c | 2 | ||||
-rw-r--r-- | firmware/lib/vboot_ui_legacy_common.c | 2 | ||||
-rw-r--r-- | firmware/lib/vboot_ui_legacy_menu.c | 2 | ||||
-rw-r--r-- | tests/vboot_api_kernel4_tests.c | 64 | ||||
-rw-r--r-- | tests/vboot_display_tests.c | 2 | ||||
-rw-r--r-- | tests/vboot_legacy_clamshell_beep_tests.c | 2 |
8 files changed, 35 insertions, 138 deletions
diff --git a/firmware/lib/include/vboot_kernel.h b/firmware/lib/include/vboot_kernel.h index c147c316..6f1a31d0 100644 --- a/firmware/lib/include/vboot_kernel.h +++ b/firmware/lib/include/vboot_kernel.h @@ -78,17 +78,4 @@ vb2_error_t VbBootDeveloperLegacyMenu(struct vb2_context *ctx); */ vb2_error_t VbBootRecoveryLegacyMenu(struct vb2_context *ctx); -/** - * Writes modified secdata spaces and nvdata. - * - * This is a temporary wrapper around vb2ex_commit_data, until secdata-writing - * functions are relocated into depthcharge. - * - * (See chromium:972956, chromium:1006689.) - * - * @param ctx Vboot context - * @returns VB2_SUCCESS, or non-zero error code. - */ -vb2_error_t vb2_commit_data(struct vb2_context *ctx); - #endif /* VBOOT_REFERENCE_VBOOT_KERNEL_H_ */ diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index c5328d23..e2c1a6c1 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -46,7 +46,7 @@ static vb2_error_t handle_battery_cutoff(struct vb2_context *ctx) vb2_nv_set(ctx, VB2_NV_BATTERY_CUTOFF_REQUEST, 0); /* May lose power immediately, so commit our update now. */ - rv = vb2_commit_data(ctx); + rv = vb2ex_commit_data(ctx); if (rv) return rv; @@ -283,55 +283,12 @@ static void vb2_kernel_fill_kparams(struct vb2_context *ctx, sizeof(kparams->partition_guid)); } -vb2_error_t vb2_commit_data(struct vb2_context *ctx) -{ - vb2_error_t rv = vb2ex_commit_data(ctx); - - switch (rv) { - case VB2_SUCCESS: - break; - - case VB2_ERROR_SECDATA_FIRMWARE_WRITE: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_W_ERROR, rv); - /* Run again to set recovery reason in nvdata. */ - vb2ex_commit_data(ctx); - return rv; - } - break; - - case VB2_ERROR_SECDATA_KERNEL_WRITE: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - vb2api_fail(ctx, VB2_RECOVERY_RW_TPM_W_ERROR, rv); - /* Run again to set recovery reason in nvdata. */ - vb2ex_commit_data(ctx); - return rv; - } - break; - - default: - VB2_DEBUG("unknown commit error: %#x\n", rv); - __attribute__ ((fallthrough)); - - case VB2_ERROR_NV_WRITE: - /* - * We can't write to nvdata, so it's impossible to - * trigger recovery mode. Skip calling vb2api_fail - * and just die (unless already in recovery). - */ - VB2_REC_OR_DIE(ctx, "write nvdata failed\n"); - break; - } - - return VB2_SUCCESS; -} - vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, VbSharedDataHeader *shared, VbSelectAndLoadKernelParams *kparams) { struct vb2_shared_data *sd = vb2_get_sd(ctx); - vb2_error_t rv, call_rv; + vb2_error_t rv; /* Init nvstorage space. TODO(kitching): Remove once we add assertions to vb2_nv_get and vb2_nv_set. */ @@ -339,11 +296,11 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, rv = vb2_kernel_setup(ctx, shared, kparams); if (rv) - goto VbSelectAndLoadKernel_exit; + return rv; rv = vb2api_kernel_phase1(ctx); if (rv) - goto VbSelectAndLoadKernel_exit; + return rv; VB2_DEBUG("GBB flags are %#x\n", vb2_get_gbb(ctx)->flags); @@ -354,35 +311,34 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { rv = vb2api_ec_sync(ctx); if (rv) - goto VbSelectAndLoadKernel_exit; + return rv; rv = vb2api_auxfw_sync(ctx); if (rv) - goto VbSelectAndLoadKernel_exit; + return rv; rv = handle_battery_cutoff(ctx); if (rv) - goto VbSelectAndLoadKernel_exit; + return rv; } /* Select boot path */ if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) { vb2_clear_recovery(ctx); - /* - * Need to commit nvdata changes immediately, since we will be - * entering either manual recovery UI or BROKEN screen shortly. - */ - vb2_commit_data(ctx); - /* If we're in recovery mode just to do memory retraining, all we need to do is reboot. */ if (sd->recovery_reason == VB2_RECOVERY_TRAIN_AND_REBOOT) { VB2_DEBUG("Reboot after retraining in recovery\n"); - rv = VBERROR_REBOOT_REQUIRED; - goto VbSelectAndLoadKernel_exit; + return VBERROR_REBOOT_REQUIRED; } + /* + * Need to commit nvdata changes immediately, since we will be + * entering either manual recovery UI or BROKEN screen shortly. + */ + vb2ex_commit_data(ctx); + /* Recovery boot. This has UI. */ if (LEGACY_MENU_UI) rv = VbBootRecoveryLegacyMenu(ctx); @@ -416,21 +372,15 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, rv = VbBootNormal(ctx); } - VbSelectAndLoadKernel_exit: + /* No need to fill kparams or convert vboot1 flags on failure. */ + if (rv) + return rv; - if (rv == VB2_SUCCESS) - vb2_kernel_fill_kparams(ctx, kparams); + vb2_kernel_fill_kparams(ctx, kparams); /* Translate vboot2 flags and fields into vboot1. */ if (sd->flags & VB2_SD_FLAG_KERNEL_SIGNED) sd->vbsd->flags |= VBSD_KERNEL_KEY_VERIFIED; - /* Commit data, but retain any previous errors */ - call_rv = vb2_commit_data(ctx); - if (rv == VB2_SUCCESS) - rv = call_rv; - - /* Pass through return value from boot path */ - VB2_DEBUG("Returning %#x\n", rv); return rv; } diff --git a/firmware/lib/vboot_display.c b/firmware/lib/vboot_display.c index 40f5bb66..3a5f602a 100644 --- a/firmware/lib/vboot_display.c +++ b/firmware/lib/vboot_display.c @@ -406,7 +406,7 @@ vb2_error_t VbCheckDisplayKey(struct vb2_context *ctx, uint32_t key, */ if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && !vb2_allow_recovery(ctx)) - vb2_commit_data(ctx); + vb2ex_commit_data(ctx); /* Force redraw of current screen */ return VbDisplayScreen(ctx, disp_current_screen, 1, data); diff --git a/firmware/lib/vboot_ui_legacy_common.c b/firmware/lib/vboot_ui_legacy_common.c index 8b6a1799..948b63e2 100644 --- a/firmware/lib/vboot_ui_legacy_common.c +++ b/firmware/lib/vboot_ui_legacy_common.c @@ -68,7 +68,7 @@ void vb2_try_altfw(struct vb2_context *ctx, int allowed, return; } - if (vb2_commit_data(ctx)) { + if (vb2ex_commit_data(ctx)) { vb2_error_notify("Error committing data on legacy boot.\n", NULL, VB_BEEP_FAILED); return; diff --git a/firmware/lib/vboot_ui_legacy_menu.c b/firmware/lib/vboot_ui_legacy_menu.c index f441b410..b0cfb43b 100644 --- a/firmware/lib/vboot_ui_legacy_menu.c +++ b/firmware/lib/vboot_ui_legacy_menu.c @@ -319,7 +319,7 @@ static vb2_error_t language_action(struct vb2_context *ctx) */ if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && !vb2_allow_recovery(ctx)) - vb2_commit_data(ctx); + vb2ex_commit_data(ctx); /* Return to previous menu. */ switch (prev_menu) { diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c index 8b736508..7c314d0f 100644 --- a/tests/vboot_api_kernel4_tests.c +++ b/tests/vboot_api_kernel4_tests.c @@ -35,7 +35,6 @@ static struct vb2_gbb_header gbb; static uint32_t kernel_version; static uint32_t new_version; static vb2_error_t vbboot_retval; -static vb2_error_t commit_data_retval; static int commit_data_called; static vb2_error_t secdata_kernel_init_retval; static vb2_error_t secdata_fwmp_init_retval; @@ -70,7 +69,6 @@ static void reset_common_data(void) memset(&shared_data, 0, sizeof(shared_data)); kernel_version = new_version = 0x10002; - commit_data_retval = VB2_SUCCESS; vbboot_retval = VB2_SUCCESS; secdata_kernel_init_retval = VB2_SUCCESS; secdata_fwmp_init_retval = VB2_SUCCESS; @@ -90,9 +88,8 @@ static void test_slk(vb2_error_t retval, int recovery_reason, const char *desc) expected_recovery_reason = recovery_reason; TEST_EQ(VbSelectAndLoadKernel(ctx, shared, &kparams), retval, desc); - TEST_EQ(current_recovery_reason, expected_recovery_reason, - " recovery reason"); - TEST_TRUE(commit_data_called, " commit nvdata"); + TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST), + recovery_reason, " recovery reason"); } /* Mock functions */ @@ -114,7 +111,7 @@ vb2_error_t vb2ex_commit_data(struct vb2_context *c) { current_recovery_reason = vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST); commit_data_called = 1; - return commit_data_retval; + return VB2_SUCCESS; } vb2_error_t vb2_secdata_kernel_init(struct vb2_context *c) @@ -158,7 +155,7 @@ vb2_error_t VbBootRecoveryLegacyClamshell(struct vb2_context *c) { TEST_EQ(current_recovery_reason, expected_recovery_reason, " recovery reason"); - TEST_TRUE(commit_data_called, " commit nvdata"); + TEST_TRUE(commit_data_called, " commit data"); shared->kernel_version_tpm = new_version; @@ -201,6 +198,7 @@ static void select_and_load_kernel_tests(void) TEST_EQ(kernel_version, 0x10002, " version"); TEST_NEQ(sd->flags & VB2_SD_STATUS_EC_SYNC_COMPLETE, 0, " EC sync complete"); + TEST_FALSE(commit_data_called, " no commit data"); /* Check EC sync toggling */ reset_common_data(); @@ -239,13 +237,6 @@ static void select_and_load_kernel_tests(void) test_slk(0, 0, "Max roll forward can't rollback"); TEST_EQ(kernel_version, 0x10002, " version"); - - reset_common_data(); - new_version = 0x20003; - commit_data_retval = VB2_ERROR_SECDATA_KERNEL_WRITE; - test_slk(VB2_ERROR_SECDATA_KERNEL_WRITE, - VB2_RECOVERY_RW_TPM_W_ERROR, "Write kernel rollback"); - /* Boot normal */ reset_common_data(); vbboot_retval = -1; @@ -261,8 +252,6 @@ static void select_and_load_kernel_tests(void) "Normal boot with diag"); TEST_EQ(vb2_nv_get(ctx, VB2_NV_DIAG_REQUEST), 0, " diag not requested"); - TEST_TRUE(commit_data_called, - " didn't commit nvdata"); } /* Boot normal - phase1 failure */ @@ -270,26 +259,12 @@ static void select_and_load_kernel_tests(void) kernel_phase1_retval = VB2_ERROR_MOCK; test_slk(VB2_ERROR_MOCK, 0, "Normal phase1 failure"); - /* Boot normal - commit data failures */ - reset_common_data(); - commit_data_retval = VB2_ERROR_SECDATA_FIRMWARE_WRITE; - test_slk(commit_data_retval, VB2_RECOVERY_RW_TPM_W_ERROR, - "Normal secdata_firmware write error triggers recovery"); - commit_data_retval = VB2_ERROR_SECDATA_KERNEL_WRITE; - test_slk(commit_data_retval, VB2_RECOVERY_RW_TPM_W_ERROR, - "Normal secdata_kernel write error triggers recovery"); - commit_data_retval = VB2_ERROR_NV_WRITE; - TEST_ABORT(VbSelectAndLoadKernel(ctx, shared, &kparams), - "Normal nvdata write error aborts"); - commit_data_retval = VB2_ERROR_UNKNOWN; - TEST_ABORT(VbSelectAndLoadKernel(ctx, shared, &kparams), - "Normal unknown commit error aborts"); - /* Boot dev */ reset_common_data(); ctx->flags |= VB2_CONTEXT_DEVELOPER_MODE; vbboot_retval = -2; test_slk(VB2_ERROR_MOCK, 0, "Dev boot bad"); + TEST_FALSE(commit_data_called, " no commit data"); reset_common_data(); ctx->flags |= VB2_CONTEXT_DEVELOPER_MODE; @@ -308,50 +283,34 @@ static void select_and_load_kernel_tests(void) sd->recovery_reason = 123; vbboot_retval = -3; test_slk(VB2_ERROR_MOCK, 0, "Recovery boot bad"); + TEST_TRUE(commit_data_called, " commit data"); reset_common_data(); sd->recovery_reason = 123; new_version = 0x20003; test_slk(0, 0, "Recovery doesn't roll forward"); TEST_EQ(kernel_version, 0x10002, " version"); + TEST_TRUE(commit_data_called, " commit data"); /* Boot recovery - phase1 failure */ reset_common_data(); sd->recovery_reason = 123; kernel_phase1_retval = VB2_ERROR_MOCK; test_slk(VB2_ERROR_MOCK, 0, "Recovery phase1 failure"); - - /* Boot recovery - commit data failures */ - reset_common_data(); - sd->recovery_reason = 123; - commit_data_retval = VB2_ERROR_SECDATA_FIRMWARE_WRITE; - test_slk(0, 0, "Recovery ignore secdata_firmware write error"); - - reset_common_data(); - sd->recovery_reason = 123; - commit_data_retval = VB2_ERROR_SECDATA_KERNEL_WRITE; - test_slk(0, 0, "Recovery ignore secdata_kernel write error"); - - reset_common_data(); - sd->recovery_reason = 123; - commit_data_retval = VB2_ERROR_NV_WRITE; - test_slk(0, 0, "Recovery return nvdata write error"); - - reset_common_data(); - sd->recovery_reason = 123; - commit_data_retval = VB2_ERROR_UNKNOWN; - test_slk(0, 0, "Recovery return unknown write error"); + TEST_FALSE(commit_data_called, " no commit data"); /* Boot recovery - memory retraining */ reset_common_data(); sd->recovery_reason = VB2_RECOVERY_TRAIN_AND_REBOOT; test_slk(VBERROR_REBOOT_REQUIRED, 0, "Recovery train and reboot"); + TEST_FALSE(commit_data_called, " no commit data"); /* Boot BROKEN recovery */ reset_common_data(); sd->recovery_reason = 123; vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13); test_slk(0, 0, "BROKEN recovery"); + TEST_TRUE(commit_data_called, " commit data"); /* Boot manual recovery */ reset_common_data(); @@ -359,6 +318,7 @@ static void select_and_load_kernel_tests(void) vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13); sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY; test_slk(0, 0, "Manual recovery"); + TEST_TRUE(commit_data_called, " commit data"); } int main(void) diff --git a/tests/vboot_display_tests.c b/tests/vboot_display_tests.c index 970025b9..a640556c 100644 --- a/tests/vboot_display_tests.c +++ b/tests/vboot_display_tests.c @@ -65,7 +65,7 @@ vb2_error_t VbExDisplayDebugInfo(const char *info_str, int full_info) return VB2_SUCCESS; } -vb2_error_t vb2_commit_data(struct vb2_context *c) +vb2_error_t vb2ex_commit_data(struct vb2_context *c) { return VB2_SUCCESS; } diff --git a/tests/vboot_legacy_clamshell_beep_tests.c b/tests/vboot_legacy_clamshell_beep_tests.c index 5edd3d19..a8c00d78 100644 --- a/tests/vboot_legacy_clamshell_beep_tests.c +++ b/tests/vboot_legacy_clamshell_beep_tests.c @@ -143,7 +143,7 @@ struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c) return &gbb; } -vb2_error_t vb2_commit_data(struct vb2_context *c) +vb2_error_t vb2ex_commit_data(struct vb2_context *c) { return VB2_SUCCESS; } |