diff options
author | Hsuan Ting Chen <roccochen@chromium.org> | 2021-08-26 14:37:37 +0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2021-12-30 06:47:13 +0000 |
commit | e4e05a30f88c76e044e5a93955627dcfdcd55974 (patch) | |
tree | b4ac1686154c6e426ebca642202a8f77dd2b27f7 | |
parent | 32a228f14660efa611d07ab5e2f38d3ba56242cf (diff) | |
download | vboot-e4e05a30f88c76e044e5a93955627dcfdcd55974.tar.gz |
2lib: Deprecate vb2api_allow_recovery() and VB2_SD_FLAG_MANUAL_RECOVERY
2lib used vb2_api_allow_recovery() to differentiate between manual and
non-manual recovery in 2kernel and UI related areas.
With introducing the ctx->boot_mode, we could decide if it is a manual
recovery or a broken screen (a.k.a non-manual recovery in the original
design) once in vb2api_fw_phase1 and use this boot mode instead for
further justifications.
Also deprecate the sd flag VB2_SD_FLAG_MANUAL_RECOVERY and use the boot
mode instead to determine if it is a manual recovery boot.
BUG=b:181931817
BRANCH=none
TEST=make clean && make runtests
TEST=emerge coreboot vboot_reference depthcharge
Cq-Depend: chromium:3282875
Signed-off-by: Hsuan Ting Chen <roccochen@chromium.org>
Change-Id: Ief4ff6cf82285c5857f0051c1f348ad0f269b4a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3121926
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
-rw-r--r-- | firmware/2lib/2kernel.c | 2 | ||||
-rw-r--r-- | firmware/2lib/2misc.c | 50 | ||||
-rw-r--r-- | firmware/2lib/include/2api.h | 13 | ||||
-rw-r--r-- | firmware/2lib/include/2struct.h | 10 | ||||
-rw-r--r-- | tests/vb2_kernel_tests.c | 11 | ||||
-rw-r--r-- | tests/vb2_misc_tests.c | 32 | ||||
-rw-r--r-- | tests/vboot_api_kernel4_tests.c | 2 |
7 files changed, 48 insertions, 72 deletions
diff --git a/firmware/2lib/2kernel.c b/firmware/2lib/2kernel.c index 86fb2863..5b18cad5 100644 --- a/firmware/2lib/2kernel.c +++ b/firmware/2lib/2kernel.c @@ -164,7 +164,7 @@ vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx) /* Load recovery key from GBB. */ rv = vb2_gbb_read_recovery_key(ctx, &packed_key, NULL, &wb); if (rv) { - if (vb2api_allow_recovery(ctx)) + if (ctx->boot_mode != VB2_BOOT_MODE_BROKEN_SCREEN) VB2_DIE("GBB read recovery key failed.\n"); else /* diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c index c1d860f1..6e926905 100644 --- a/firmware/2lib/2misc.c +++ b/firmware/2lib/2misc.c @@ -154,7 +154,6 @@ void vb2_check_recovery(struct vb2_context *ctx) else /* Recovery was forced. Override recovery reason */ sd->recovery_reason = VB2_RECOVERY_RO_MANUAL; - sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY; } /* If recovery reason is non-zero, tell caller we need recovery mode */ @@ -379,7 +378,7 @@ vb2_error_t vb2_select_fw_slot(struct vb2_context *ctx) vb2_error_t vb2api_enable_developer_mode(struct vb2_context *ctx) { - if (!vb2api_allow_recovery(ctx)) { + if (ctx->boot_mode != VB2_BOOT_MODE_MANUAL_RECOVERY) { VB2_DEBUG("ERROR: Can only enable developer mode from manual " "recovery mode\n"); return VB2_ERROR_API_ENABLE_DEV_NOT_ALLOWED; @@ -415,30 +414,6 @@ void vb2api_request_diagnostics(struct vb2_context *ctx) { VB2_DEBUG("Diagnostics requested\n"); } -test_mockable -int vb2api_allow_recovery(struct vb2_context *ctx) -{ - if (ctx->flags & VB2_CONTEXT_NO_BOOT) - return 0; - - /* VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY forces this to always return - true. */ - if (vb2_get_gbb(ctx)->flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY) - return 1; - - /* - * If EC is in RW, it implies recovery wasn't manually requested. - * On some platforms, EC_IN_RW can't be reset by the EC, thus, this may - * return false (=RW). That's ok because if recovery is manual, we will - * get the right signal and that's the case we care about. - */ - if (!(ctx->flags & VB2_CONTEXT_EC_TRUSTED)) - return 0; - - /* Now we confidently check the recovery switch state at boot */ - return !!(vb2_get_sd(ctx)->flags & VB2_SD_FLAG_MANUAL_RECOVERY); -} - void vb2_clear_recovery(struct vb2_context *ctx) { struct vb2_shared_data *sd = vb2_get_sd(ctx); @@ -450,13 +425,13 @@ void vb2_clear_recovery(struct vb2_context *ctx) reason, subcode, vb2_get_recovery_reason_string(reason)); - /* Clear recovery request for both manual and non-manual. */ + /* Clear recovery request for both the manual recovery and the broken + screen. */ vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, VB2_RECOVERY_NOT_REQUESTED); vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 0); - /* But stow recovery reason as subcode for non-manual recovery. */ - if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && - !vb2api_allow_recovery(ctx)) { + /* But stow recovery reason as subcode for the broken screen. */ + if (ctx->boot_mode == VB2_BOOT_MODE_BROKEN_SCREEN) { VB2_DEBUG("Stow recovery reason as subcode (%#x)\n", sd->recovery_reason); vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, sd->recovery_reason); @@ -737,13 +712,24 @@ char *vb2api_get_debug_info(struct vb2_context *ctx) void vb2_set_boot_mode(struct vb2_context *ctx) { struct vb2_shared_data *sd = vb2_get_sd(ctx); + struct vb2_gbb_header *gbb = vb2_get_gbb(ctx); /* Cast boot mode to non-constant and assign */ enum vb2_boot_mode *boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode; *boot_mode = VB2_BOOT_MODE_NORMAL; - if (sd->recovery_reason) { - if (vb2api_allow_recovery(ctx)) + /* + * The only way to pass this check and proceed to the recovery process + * is to physically request a recovery (a.k.a. manual recovery). All + * other recovery requests including manual recovery requested by a + * (compromised) host will end up with 'broken' screen. + */ + if ((ctx->flags & VB2_CONTEXT_FORCE_RECOVERY_MODE) && + !(ctx->flags & VB2_CONTEXT_NO_BOOT) && + (ctx->flags & VB2_CONTEXT_EC_TRUSTED)) { + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; + } else if (sd->recovery_reason) { + if (gbb->flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY) *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; else *boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN; diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h index cd558b05..14301111 100644 --- a/firmware/2lib/include/2api.h +++ b/firmware/2lib/include/2api.h @@ -1012,19 +1012,6 @@ enum vb2_dev_default_boot_target vb2api_get_dev_default_boot_target( int vb2api_use_short_dev_screen_delay(struct vb2_context *ctx); /** - * Check whether recovery is allowed or not. - * - * The only way to pass this check and proceed to the recovery process is to - * physically request a recovery (a.k.a. manual recovery). All other recovery - * requests including manual recovery requested by a (compromised) host will - * end up with 'broken' screen. - * - * @param ctx Vboot context - * @return 1 if recovery is allowed; 0 if no or uncertain. - */ -int vb2api_allow_recovery(struct vb2_context *ctx); - -/** * Request to enable developer mode. * * Enables the developer flag in vb2_context firmware secdata. Note that diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h index 310f4bc5..3e2c4228 100644 --- a/firmware/2lib/include/2struct.h +++ b/firmware/2lib/include/2struct.h @@ -23,8 +23,14 @@ /* Flags for vb2_shared_data.flags */ enum vb2_shared_data_flags { - /* User has explicitly and physically requested recovery */ - VB2_SD_FLAG_MANUAL_RECOVERY = (1 << 0), + /* + * VB2_SD_FLAG_MANUAL_RECOVERY (1 << 0) is deprecated. This flag is not + * necessary since the introduction of persistent context (CL:1716351). + * With introducing vb2_boot_mode and differentiating between manual + * recovery and broken screen (CL:3274699), it is suggested to leverage + * the vb2_boot_mode instead to determine if the user has physically + * requested recovery. + */ /* Developer mode is enabled */ VB2_SD_FLAG_DEV_MODE_ENABLED = (1 << 1), diff --git a/tests/vb2_kernel_tests.c b/tests/vb2_kernel_tests.c index 0dc0e749..0b3e94c7 100644 --- a/tests/vb2_kernel_tests.c +++ b/tests/vb2_kernel_tests.c @@ -24,6 +24,7 @@ static struct vb2_context *ctx; static struct vb2_shared_data *sd; static struct vb2_fw_preamble *fwpre; static const char fw_kernel_key_data[36] = "Test kernel key data"; +static enum vb2_boot_mode *boot_mode; /* Mocked function data */ @@ -82,6 +83,14 @@ static void reset_common_data(enum reset_type t) mock_gbb.recovery_key.key_offset + mock_gbb.recovery_key.key_size; + /* For boot_mode */ + boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode; + if (t == FOR_PHASE1) + *boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN; + else if (t == FOR_NORMAL_BOOT) + *boot_mode = VB2_BOOT_MODE_NORMAL; + else + *boot_mode = VB2_BOOT_MODE_UNDEFINED; if (t == FOR_PHASE1) { uint8_t *kdata; @@ -274,6 +283,7 @@ static void phase1_tests(void) TEST_EQ(sd->kernel_key_offset, 0, " workbuf key offset"); TEST_EQ(sd->kernel_key_size, 0, " workbuf key size"); mock_gbb.h.flags |= VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY; + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; TEST_ABORT(vb2api_kernel_phase1(ctx), " fatal for manual recovery"); reset_common_data(FOR_PHASE1); @@ -284,6 +294,7 @@ static void phase1_tests(void) TEST_EQ(sd->kernel_key_offset, 0, " workbuf key offset"); TEST_EQ(sd->kernel_key_size, 0, " workbuf key size"); mock_gbb.h.flags |= VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY; + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; mock_read_res_fail_on_call = 1; TEST_ABORT(vb2api_kernel_phase1(ctx), " fatal for manual recovery"); diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c index c552301e..99f8bae4 100644 --- a/tests/vb2_misc_tests.c +++ b/tests/vb2_misc_tests.c @@ -22,6 +22,7 @@ static struct vb2_context *ctx; static struct vb2_shared_data *sd; static struct vb2_gbb_header gbb; static struct vb2_secdata_fwmp *fwmp; +static enum vb2_boot_mode *boot_mode; /* Mocked function data */ static enum vb2_resource_index mock_resource_index; @@ -29,7 +30,6 @@ static void *mock_resource_ptr; static uint32_t mock_resource_size; static int mock_tpm_clear_called; static int mock_tpm_clear_retval; -static int allow_recovery_retval; static void reset_common_data(void) { @@ -53,16 +53,13 @@ static void reset_common_data(void) mock_tpm_clear_called = 0; mock_tpm_clear_retval = VB2_SUCCESS; - allow_recovery_retval = 0; + + boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode; + *boot_mode = VB2_BOOT_MODE_NORMAL; }; /* Mocked functions */ -int vb2api_allow_recovery(struct vb2_context *c) -{ - return allow_recovery_retval; -} - struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c) { return &gbb; @@ -430,8 +427,6 @@ static void recovery_tests(void) 0, "recovery not yet decided before testing check_recovery()"); vb2_check_recovery(ctx); TEST_EQ(sd->recovery_reason, 0, "No recovery reason"); - TEST_EQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY, - 0, "Not manual recovery"); TEST_EQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, "Not recovery mode"); TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED, @@ -443,8 +438,6 @@ static void recovery_tests(void) vb2_check_recovery(ctx); TEST_EQ(sd->recovery_reason, 3, "Recovery reason from request"); TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST), 3, "NV not cleared"); - TEST_EQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY, - 0, "Not manual recovery"); TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, "Recovery mode"); TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED, @@ -468,8 +461,6 @@ static void recovery_tests(void) vb2_check_recovery(ctx); TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_MANUAL, "Recovery reason forced"); - TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY, - 0, "SD flag set"); TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED, 0, "Recovery decided"); @@ -480,8 +471,6 @@ static void recovery_tests(void) vb2_check_recovery(ctx); TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_MANUAL, "Recovery reason forced"); - TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY, - 0, "SD flag set"); TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED, 0, "Recovery decided"); @@ -492,8 +481,6 @@ static void recovery_tests(void) vb2_check_recovery(ctx); TEST_EQ(sd->recovery_reason, VB2_RECOVERY_US_TEST, "Recovery reason forced from BROKEN"); - TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY, - 0, "SD flag set"); TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED, 0, "Recovery decided"); } @@ -661,7 +648,6 @@ static void dev_switch_tests(void) static void enable_dev_tests(void) { reset_common_data(); - allow_recovery_retval = 0; TEST_FAIL(vb2api_enable_developer_mode(ctx), "vb2api_enable_developer_mode - failed"); TEST_EQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) & @@ -669,7 +655,7 @@ static void enable_dev_tests(void) " dev mode flag not set"); reset_common_data(); - allow_recovery_retval = 1; + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; TEST_SUCC(vb2api_enable_developer_mode(ctx), "vb2api_enable_developer_mode - success"); TEST_NEQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) & @@ -678,7 +664,7 @@ static void enable_dev_tests(void) /* secdata_firmware not initialized, aborts */ reset_common_data(); - allow_recovery_retval = 1; + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; sd->status &= ~VB2_SD_STATUS_SECDATA_FIRMWARE_INIT; sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED; TEST_ABORT(vb2api_enable_developer_mode(ctx), @@ -830,7 +816,7 @@ static void clear_recovery_tests(void) /* Manual recovery */ reset_common_data(); - allow_recovery_retval = 1; + *boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY; sd->recovery_reason = 4; ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5); @@ -841,9 +827,9 @@ static void clear_recovery_tests(void) TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE), 0, " subcode cleared"); - /* BROKEN recovery */ + /* Broken screen */ reset_common_data(); - allow_recovery_retval = 0; + *boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN; sd->recovery_reason = 4; ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5); diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c index c1e71afa..e3f19a8c 100644 --- a/tests/vboot_api_kernel4_tests.c +++ b/tests/vboot_api_kernel4_tests.c @@ -328,7 +328,7 @@ static void select_and_load_kernel_tests(void) reset_common_data(); sd->recovery_reason = VB2_RECOVERY_RO_MANUAL; vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13); - sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY; + ctx->flags &= VB2_CONTEXT_FORCE_RECOVERY_MODE; test_slk(0, 0, "Manual recovery"); TEST_TRUE(commit_data_called, " commit data"); } |