From 9df04f353c23a9a45cde0321e441e7dcddf5377f Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Tue, 4 Feb 2020 16:39:21 -0800 Subject: firmware: Add VB2_REC_OR_DIE() helper macro After introducing VB2_DIE() recently, practical use has shown that we pretty much always want to check for recovery mode first, and avoid a hard abort in that case. This patch introduces a very similar macro that includes that extra check so we don't have to open-code it all over the place. BRANCH=None BUG=None TEST=make runtests Change-Id: I16e744859ba7a5c68269e06c7e7d071f3bfae67e Signed-off-by: Julius Werner Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2038244 Reviewed-by: Joel Kitching --- firmware/2lib/2secdata_firmware.c | 8 ++------ firmware/2lib/2secdata_fwmp.c | 17 ++++------------- firmware/2lib/2secdata_kernel.c | 8 ++------ firmware/2lib/include/2common.h | 9 +++++++++ firmware/lib/vboot_api_kernel.c | 13 ++++++------- tests/vb2_misc_tests.c | 8 ++++++++ 6 files changed, 31 insertions(+), 32 deletions(-) diff --git a/firmware/2lib/2secdata_firmware.c b/firmware/2lib/2secdata_firmware.c index 44cd9e74..98e65b05 100644 --- a/firmware/2lib/2secdata_firmware.c +++ b/firmware/2lib/2secdata_firmware.c @@ -97,9 +97,7 @@ uint32_t vb2_secdata_firmware_get(struct vb2_context *ctx, } fail: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) - VB2_DIE("%s\n", msg); - VB2_DEBUG("ERROR [%s] ignored in recovery mode\n", msg); + VB2_REC_OR_DIE(ctx, "%s\n", msg); return 0; } @@ -152,7 +150,5 @@ void vb2_secdata_firmware_set(struct vb2_context *ctx, return; fail: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) - VB2_DIE("%s\n", msg); - VB2_DEBUG("ERROR [%s] ignored in recovery mode\n", msg); + VB2_REC_OR_DIE(ctx, "%s\n", msg); } diff --git a/firmware/2lib/2secdata_fwmp.c b/firmware/2lib/2secdata_fwmp.c index 7d19fca4..170fc5c2 100644 --- a/firmware/2lib/2secdata_fwmp.c +++ b/firmware/2lib/2secdata_fwmp.c @@ -87,12 +87,8 @@ int vb2_secdata_fwmp_get_flag(struct vb2_context *ctx, (struct vb2_secdata_fwmp *)&ctx->secdata_fwmp; if (!(sd->status & VB2_SD_STATUS_SECDATA_FWMP_INIT)) { - if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) { - VB2_DEBUG("Assuming broken FWMP flag %d as 0\n", flag); - return 0; - } else { - VB2_DIE("Must init FWMP before retrieving flag\n"); - } + VB2_REC_OR_DIE(ctx, "Must init FWMP before retrieving flag\n"); + return 0; } if (ctx->flags & VB2_CONTEXT_NO_SECDATA_FWMP) @@ -108,13 +104,8 @@ uint8_t *vb2_secdata_fwmp_get_dev_key_hash(struct vb2_context *ctx) (struct vb2_secdata_fwmp *)&ctx->secdata_fwmp; if (!(sd->status & VB2_SD_STATUS_SECDATA_FWMP_INIT)) { - if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) { - VB2_DEBUG("Assuming broken FWMP dev_key_hash " - "as empty\n"); - return NULL; - } else { - VB2_DIE("Must init FWMP before getting dev key hash\n"); - } + VB2_REC_OR_DIE(ctx, "Must init FWMP before get dev key hash\n"); + return NULL; } if (ctx->flags & VB2_CONTEXT_NO_SECDATA_FWMP) diff --git a/firmware/2lib/2secdata_kernel.c b/firmware/2lib/2secdata_kernel.c index 4764ace1..ca122ab5 100644 --- a/firmware/2lib/2secdata_kernel.c +++ b/firmware/2lib/2secdata_kernel.c @@ -99,9 +99,7 @@ uint32_t vb2_secdata_kernel_get(struct vb2_context *ctx, } fail: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) - VB2_DIE("%s\n", msg); - VB2_DEBUG("ERROR [%s] ignored in recovery mode\n", msg); + VB2_REC_OR_DIE(ctx, "%s\n", msg); return 0; } @@ -141,7 +139,5 @@ void vb2_secdata_kernel_set(struct vb2_context *ctx, return; fail: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) - VB2_DIE("%s\n", msg); - VB2_DEBUG("ERROR [%s] ignored in recovery mode\n", msg); + VB2_REC_OR_DIE(ctx, "%s\n", msg); } diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index 5470d2a6..135beb96 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -59,6 +59,15 @@ struct vb2_public_key; for (;;); \ } while (0) +#define VB2_REC_OR_DIE(ctx, format, args...) do { \ + VB2_DEBUG(format, ## args); \ + if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { \ + vb2ex_abort(); \ + for (;;); \ + } \ + VB2_DEBUG("IGNORING ABORT IN RECOVERY MODE!!!\n"); \ +} while (0) + /* * Define test_mockable and for mocking functions when compiled for Chrome OS * environment (that is, not for firmware). diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index 0e2e8fb3..1c7d974c 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -321,13 +321,12 @@ vb2_error_t vb2_commit_data(struct vb2_context *ctx) __attribute__ ((fallthrough)); case VB2_ERROR_NV_WRITE: - if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) - /* - * We can't write to nvdata, so it's impossible to - * trigger recovery mode. Skip calling vb2api_fail - * and just die. - */ - VB2_DIE("write nvdata failed\n"); + /* + * 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; } diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c index 2a3f88dc..b3da6454 100644 --- a/tests/vb2_misc_tests.c +++ b/tests/vb2_misc_tests.c @@ -255,6 +255,14 @@ static void misc_tests(void) "vb_workbuf_from_ctx() buf"); TEST_EQ(wb.size, sd->workbuf_size - VB2_WORKBUF_ALIGN, "vb_workbuf_from_ctx() size"); + + reset_common_data(); + TEST_ABORT(VB2_REC_OR_DIE(ctx, "die\n"), "REC_OR_DIE in normal mode"); + + reset_common_data(); + ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; + VB2_REC_OR_DIE(ctx, "VB2_REC_OR_DIE() test in recovery mode\n"); + /* Would exit here if it didn't work as intended. */ } static void gbb_tests(void) -- cgit v1.2.1