diff options
| author | Julius Werner <jwerner@chromium.org> | 2021-11-25 13:52:05 -0800 |
|---|---|---|
| committer | Commit Bot <commit-bot@chromium.org> | 2021-12-03 01:53:37 +0000 |
| commit | dd180f6d8545eace4ccc4569c32dbf7bff0354f5 (patch) | |
| tree | 19d6333e36da63b8326eadfe8419883573e0c25f /firmware | |
| parent | afd4e0ae05efda36347a406f429ceb4952faf0a0 (diff) | |
| download | vboot-dd180f6d8545eace4ccc4569c32dbf7bff0354f5.tar.gz | |
firmware: VB2_REC_OR_DIE() should not abort before vb2_check_recovery()
Unfortunately, CL:3168437 introduced a new problem when booting with a
broken TPM: secdata accessors no longer return failure but instead just
abort when booting in normal mode and continue when we're in recovery
mode. The problem is that when accessing secdata very early in
vb2api_fw_phase1(), we have not decided whether we're booting in
recovery mode yet. If vb2_secdata_firmware_init() fails, we will call
vb2api_fail() and then continue knowing that vb2_check_recovery() will
later see the recovery reason in NVRAM and decide to boot directly into
recovery from here. But if the code in-between accesses secdata, the
VB2_CONTEXT_RECOVERY_MODE flag is technically not yet set, so our
secdata accessor thinks we are booting in normal mode and something
terrible happened (because it shouldn't be possible to boot in normal
mode when secdata_init failed), so it aborts.
In order to try to solve this problem in a more general way, introduce a
new VB2_SD_STATUS_RECOVERY_DECIDED status flag that gets set once we
reach the point where we have conclusively decided whether we are
booting into recovery mode and set the appropriate context flags. Any
code using VB2_REC_OR_DIE() before that point will play it safe and
assume that we may still go into recovery mode, so we shouldn't abort.
BRANCH=none
BUG=none
TEST=none
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ic3daa8dac932286257cbceebfff8712d25c3a97a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3301540
Reviewed-by: Hsuan Ting Chen <roccochen@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Diffstat (limited to 'firmware')
| -rw-r--r-- | firmware/2lib/2misc.c | 2 | ||||
| -rw-r--r-- | firmware/2lib/include/2common.h | 4 | ||||
| -rw-r--r-- | firmware/2lib/include/2struct.h | 2 |
3 files changed, 7 insertions, 1 deletions
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c index 265db317..4896694b 100644 --- a/firmware/2lib/2misc.c +++ b/firmware/2lib/2misc.c @@ -164,6 +164,8 @@ void vb2_check_recovery(struct vb2_context *ctx) sd->recovery_reason, vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE)); } + + sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED; } vb2_error_t vb2_fw_init_gbb(struct vb2_context *ctx) diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index 695f50d1..d43f18a5 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -10,6 +10,7 @@ #include "2api.h" #include "2gbb.h" +#include "2misc.h" #include "2packed_key.h" #include "2return_codes.h" #include "2sha.h" @@ -66,7 +67,8 @@ struct vb2_public_key; #define VB2_REC_OR_DIE(ctx, format, args...) do { \ VB2_DEBUG(format, ## args); \ - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { \ + if ((vb2_get_sd(ctx)->status & VB2_SD_STATUS_RECOVERY_DECIDED) && \ + !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { \ vb2ex_abort(); \ for (;;); \ } \ diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h index ea193d74..08a1efe3 100644 --- a/firmware/2lib/include/2struct.h +++ b/firmware/2lib/include/2struct.h @@ -85,6 +85,8 @@ enum vb2_shared_data_status { /* EC Sync completed successfully */ VB2_SD_STATUS_EC_SYNC_COMPLETE = (1 << 6), + /* Have checked whether we are booting into recovery mode or not. */ + VB2_SD_STATUS_RECOVERY_DECIDED = (1 << 7), }; /* "V2SD" = vb2_shared_data.magic */ |