vboot/ui: Refactor ui error handling
Add helper functions to check and set ui error code. The ui error
handling shouldn't catch the requests but only the error.
Add vb2_is_error() to 2api.h. This function is for checking if the
return value is an error or not.
BRANCH=none
BUG=b:157625765
TEST=make clean && CC=x86_64-pc-linux-gnu-clang make runtests
Signed-off-by: Chung-Sheng Wu <chungsheng@google.com>
Change-Id: I5c9a34dadf749f3b5364860a1a034bfefe0a61f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2780821
Tested-by: Chung-Sheng Wu <chungsheng@chromium.org>
Commit-Queue: Chung-Sheng Wu <chungsheng@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-by: Joel Kitching <kitching@chromium.org>
diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c
index 1ac50ed..a9359b5 100644
--- a/firmware/2lib/2ui_screens.c
+++ b/firmware/2lib/2ui_screens.c
@@ -53,6 +53,32 @@
/******************************************************************************/
/*
+ * Functions for ui error handling
+ */
+
+static vb2_error_t set_ui_error(struct vb2_ui_context *ui,
+ enum vb2_ui_error error_code)
+{
+ /* Keep the first occurring error. */
+ if (ui->error_code)
+ VB2_DEBUG("When handling ui error %#x, another ui error "
+ "occurred: %#x",
+ ui->error_code, error_code);
+ else
+ ui->error_code = error_code;
+ /* Return to the ui loop to show the error code. */
+ return VB2_REQUEST_UI_CONTINUE;
+}
+
+static vb2_error_t set_ui_error_and_go_back(struct vb2_ui_context *ui,
+ enum vb2_ui_error error_code)
+{
+ set_ui_error(ui, error_code);
+ return vb2_ui_screen_back(ui);
+}
+
+/******************************************************************************/
+/*
* Functions used for log screens
*
* Expects that the page_count is valid and page_up_item and page_down_item are
@@ -295,23 +321,19 @@
static vb2_error_t debug_info_set_content(struct vb2_ui_context *ui)
{
const char *log_string = vb2ex_get_debug_info(ui->ctx);
- if (!log_string) {
- VB2_DEBUG("ERROR: Failed to retrieve debug info\n");
- ui->error_code = VB2_UI_ERROR_DEBUG_LOG;
- return vb2_ui_screen_back(ui);
- }
- vb2_error_t rv = log_page_update(ui, log_string);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DEBUG_LOG;
- return vb2_ui_screen_back(ui);
- }
+ if (!log_string)
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DEBUG_LOG);
+ if (vb2_is_error(log_page_update(ui, log_string)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DEBUG_LOG);
return VB2_SUCCESS;
}
static vb2_error_t debug_info_init(struct vb2_ui_context *ui)
{
VB2_TRY(debug_info_set_content(ui));
- return log_page_reset_to_top(ui);
+ if (vb2_is_error(log_page_reset_to_top(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DEBUG_LOG);
+ return VB2_SUCCESS;
}
static vb2_error_t debug_info_reinit(struct vb2_ui_context *ui)
@@ -349,23 +371,19 @@
int reset)
{
const char *log_string = vb2ex_get_firmware_log(reset);
- if (!log_string) {
- VB2_DEBUG("ERROR: Failed to retrieve firmware log\n");
- ui->error_code = VB2_UI_ERROR_FIRMWARE_LOG;
- return vb2_ui_screen_back(ui);
- }
- vb2_error_t rv = log_page_update(ui, log_string);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_FIRMWARE_LOG;
- return vb2_ui_screen_back(ui);
- }
+ if (!log_string)
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_FIRMWARE_LOG);
+ if (vb2_is_error(log_page_update(ui, log_string)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_FIRMWARE_LOG);
return VB2_SUCCESS;
}
static vb2_error_t firmware_log_init(struct vb2_ui_context *ui)
{
VB2_TRY(firmware_log_set_content(ui, 1));
- return log_page_reset_to_top(ui);
+ if (vb2_is_error(log_page_reset_to_top(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_FIRMWARE_LOG);
+ return VB2_SUCCESS;
}
static vb2_error_t firmware_log_reinit(struct vb2_ui_context *ui)
@@ -471,11 +489,10 @@
vb2_error_t recovery_to_dev_init(struct vb2_ui_context *ui)
{
- if (vb2_get_sd(ui->ctx)->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) {
+ if (vb2_get_sd(ui->ctx)->flags & VB2_SD_FLAG_DEV_MODE_ENABLED)
/* We're in dev mode, so let user know they can't transition */
- ui->error_code = VB2_UI_ERROR_DEV_MODE_ALREADY_ENABLED;
- return vb2_ui_screen_back(ui);
- }
+ return set_ui_error_and_go_back(
+ ui, VB2_UI_ERROR_DEV_MODE_ALREADY_ENABLED);
if (!PHYSICAL_PRESENCE_KEYBOARD && vb2ex_physical_presence_pressed()) {
VB2_DEBUG("Presence button stuck?\n");
@@ -524,7 +541,8 @@
* from an untrusted keyboard.
*/
if (PHYSICAL_PRESENCE_KEYBOARD && ui->key == VB_KEY_ENTER)
- ui->error_code = VB2_UI_ERROR_UNTRUSTED_CONFIRMATION;
+ return set_ui_error(
+ ui, VB2_UI_ERROR_UNTRUSTED_CONFIRMATION);
return VB2_SUCCESS;
}
return recovery_to_dev_finalize(ui);
@@ -727,8 +745,7 @@
!vb2_dev_boot_external_allowed(ui->ctx)) {
VB2_DEBUG("ERROR: Dev mode external boot not allowed\n");
ui->error_beep = 1;
- ui->error_code = VB2_UI_ERROR_EXTERNAL_BOOT_DISABLED;
- return VB2_SUCCESS;
+ return set_ui_error(ui, VB2_UI_ERROR_EXTERNAL_BOOT_DISABLED);
}
rv = VbTryLoadKernel(ui->ctx, VB_DISK_FLAG_REMOVABLE);
@@ -831,8 +848,8 @@
/* Don't allow to-norm if GBB forces dev mode */
if (vb2_get_gbb(ui->ctx)->flags & VB2_GBB_FLAG_FORCE_DEV_SWITCH_ON) {
VB2_DEBUG("ERROR: to-norm not allowed\n");
- ui->error_code = VB2_UI_ERROR_TO_NORM_NOT_ALLOWED;
- return vb2_ui_screen_back(ui);
+ return set_ui_error_and_go_back(
+ ui, VB2_UI_ERROR_TO_NORM_NOT_ALLOWED);
}
ui->state->selected_item = DEVELOPER_TO_NORM_ITEM_CONFIRM;
/* Hide "Cancel" button if dev boot is not allowed */
@@ -920,10 +937,8 @@
static vb2_error_t developer_select_bootloader_init(struct vb2_ui_context *ui)
{
- if (get_menu(ui)->num_items == 0) {
- ui->error_code = VB2_UI_ERROR_ALTFW_EMPTY;
- return vb2_ui_screen_back(ui);
- }
+ if (get_menu(ui)->num_items == 0)
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_ALTFW_EMPTY);
/* Select the first bootloader. */
ui->state->selected_item =
ARRAY_SIZE(developer_select_bootloader_items_before);
@@ -941,14 +956,12 @@
!vb2_dev_boot_allowed(ui->ctx) ||
!vb2_dev_boot_altfw_allowed(ui->ctx)) {
VB2_DEBUG("ERROR: Dev mode alternate bootloader not allowed\n");
- ui->error_code = VB2_UI_ERROR_ALTFW_DISABLED;
- return VB2_SUCCESS;
+ return set_ui_error(ui, VB2_UI_ERROR_ALTFW_DISABLED);
}
if (vb2ex_get_altfw_count() == 0) {
VB2_DEBUG("ERROR: No alternate bootloader was found\n");
- ui->error_code = VB2_UI_ERROR_ALTFW_EMPTY;
- return VB2_SUCCESS;
+ return set_ui_error(ui, VB2_UI_ERROR_ALTFW_EMPTY);
}
if (ui->key == VB_KEY_CTRL('L')) {
@@ -963,8 +976,7 @@
vb2ex_run_altfw(altfw_id);
VB2_DEBUG("ERROR: Alternate bootloader failed\n");
- ui->error_code = VB2_UI_ERROR_ALTFW_FAILED;
- return VB2_SUCCESS;
+ return set_ui_error(ui, VB2_UI_ERROR_ALTFW_FAILED);
}
static const struct vb2_menu *get_bootloader_menu(struct vb2_ui_context *ui)
@@ -1083,22 +1095,22 @@
#define DIAGNOSTICS_STORAGE_HEALTH_ITEM_PAGE_DOWN 1
#define DIAGNOSTICS_STORAGE_HEALTH_ITEM_BACK 2
-static vb2_error_t diagnostics_storage_health_init(struct vb2_ui_context *ui)
+static vb2_error_t diagnostics_storage_health_init_impl(
+ struct vb2_ui_context *ui)
{
const char *log_string;
- vb2_error_t rv = vb2ex_diag_get_storage_health(&log_string);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DIAGNOSTICS;
- return vb2_ui_screen_back(ui);
- }
- rv = log_page_update(ui, log_string);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DIAGNOSTICS;
- return vb2_ui_screen_back(ui);
- }
+ VB2_TRY(vb2ex_diag_get_storage_health(&log_string));
+ VB2_TRY(log_page_update(ui, log_string));
return log_page_reset_to_top(ui);
}
+static vb2_error_t diagnostics_storage_health_init(struct vb2_ui_context *ui)
+{
+ if (vb2_is_error(diagnostics_storage_health_init_impl(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
+ return VB2_SUCCESS;
+}
+
static const struct vb2_menu_item diagnostics_storage_health_items[] = {
[DIAGNOSTICS_STORAGE_HEALTH_ITEM_PAGE_UP] = PAGE_UP_ITEM,
[DIAGNOSTICS_STORAGE_HEALTH_ITEM_PAGE_DOWN] = PAGE_DOWN_ITEM,
@@ -1152,29 +1164,25 @@
static vb2_error_t diagnostics_storage_test_update(struct vb2_ui_context *ui)
{
- vb2_error_t rv = diagnostics_storage_test_update_impl(ui);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DIAGNOSTICS;
- return vb2_ui_screen_back(ui);
- }
+ if (vb2_is_error(diagnostics_storage_test_update_impl(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
return VB2_SUCCESS;
}
static vb2_error_t diagnostics_storage_test_control(
struct vb2_ui_context *ui, enum vb2_diag_storage_test op)
{
- vb2_error_t rv = vb2ex_diag_storage_test_control(op);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DIAGNOSTICS;
- return vb2_ui_screen_back(ui);
- }
+ if (vb2_is_error(vb2ex_diag_storage_test_control(op)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
return VB2_SUCCESS;
}
static vb2_error_t diagnostics_storage_test_init(struct vb2_ui_context *ui)
{
VB2_TRY(diagnostics_storage_test_update(ui));
- return log_page_reset_to_top(ui);
+ if (vb2_is_error(log_page_reset_to_top(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
+ return VB2_SUCCESS;
}
static vb2_error_t diagnostics_storage_test_short_init(
@@ -1283,11 +1291,8 @@
memory_test_op_t op,
int reset)
{
- vb2_error_t rv = diagnostics_memory_update_screen_impl(ui, op, reset);
- if (rv) {
- ui->error_code = VB2_UI_ERROR_DIAGNOSTICS;
- return vb2_ui_screen_back(ui);
- }
+ if (vb2_is_error(diagnostics_memory_update_screen_impl(ui, op, reset)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
return VB2_SUCCESS;
}
@@ -1295,14 +1300,18 @@
{
VB2_TRY(diagnostics_memory_update_screen(
ui, &vb2ex_diag_memory_quick_test, 1));
- return log_page_reset_to_top(ui);
+ if (vb2_is_error(log_page_reset_to_top(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
+ return VB2_SUCCESS;
}
static vb2_error_t diagnostics_memory_init_full(struct vb2_ui_context *ui)
{
VB2_TRY(diagnostics_memory_update_screen(
ui, &vb2ex_diag_memory_full_test, 1));
- return log_page_reset_to_top(ui);
+ if (vb2_is_error(log_page_reset_to_top(ui)))
+ return set_ui_error_and_go_back(ui, VB2_UI_ERROR_DIAGNOSTICS);
+ return VB2_SUCCESS;
}
static vb2_error_t diagnostics_memory_update_quick(struct vb2_ui_context *ui)
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index f7acdc6..bd3b05f 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -64,6 +64,17 @@
*/
#define VB2_TRY(expr, ...) _VB2_TRY_IMPL(expr, ##__VA_ARGS__, NULL, 0)
+/**
+ * Check if the return value is an error.
+ *
+ * @param rv The return value.
+ * @return True if the value is an error.
+ */
+static inline int vb2_is_error(vb2_error_t rv)
+{
+ return rv >= VB2_ERROR_BASE && rv <= VB2_ERROR_MAX;
+}
+
/* Flags for vb2_context.
*
* Unless otherwise noted, flags are set by verified boot and may be read (but
diff --git a/tests/vb2_ui_action_tests.c b/tests/vb2_ui_action_tests.c
index 4254684..66a776e 100644
--- a/tests/vb2_ui_action_tests.c
+++ b/tests/vb2_ui_action_tests.c
@@ -757,7 +757,9 @@
reset_common_data();
mock_dev_boot_altfw_allowed = 1;
TEST_EQ(vb2_ui_developer_mode_boot_altfw_action(&mock_ui_context),
- VB2_SUCCESS, "not allowed: not in dev mode");
+ VB2_REQUEST_UI_CONTINUE, "not allowed: not in dev mode");
+ TEST_EQ(mock_ui_context.error_code, VB2_UI_ERROR_ALTFW_DISABLED,
+ "ui_error code is set");
TEST_EQ(mock_run_altfw_called, 0, " vb2ex_run_altfw not called");
/* Not allowed: dev boot not allowed */
@@ -766,14 +768,18 @@
mock_dev_boot_allowed = 0;
mock_dev_boot_altfw_allowed = 1;
TEST_EQ(vb2_ui_developer_mode_boot_altfw_action(&mock_ui_context),
- VB2_SUCCESS, "not allowed: dev boot not allowed");
+ VB2_REQUEST_UI_CONTINUE, "not allowed: dev boot not allowed");
+ TEST_EQ(mock_ui_context.error_code, VB2_UI_ERROR_ALTFW_DISABLED,
+ "ui_error code is set");
TEST_EQ(mock_run_altfw_called, 0, " vb2ex_run_altfw not called");
/* Not allowed: boot altfw not allowed */
reset_common_data();
ctx->flags |= VB2_CONTEXT_DEVELOPER_MODE;
TEST_EQ(vb2_ui_developer_mode_boot_altfw_action(&mock_ui_context),
- VB2_SUCCESS, "not allowed: boot altfw not allowed");
+ VB2_REQUEST_UI_CONTINUE, "not allowed: boot altfw not allowed");
+ TEST_EQ(mock_ui_context.error_code, VB2_UI_ERROR_ALTFW_DISABLED,
+ "ui_error code is set");
TEST_EQ(mock_run_altfw_called, 0, " vb2ex_run_altfw not called");
/* Allowed */
@@ -782,7 +788,9 @@
mock_dev_boot_altfw_allowed = 1;
mock_ui_context.state->selected_item = 2;
TEST_EQ(vb2_ui_developer_mode_boot_altfw_action(&mock_ui_context),
- VB2_SUCCESS, "allowed");
+ VB2_REQUEST_UI_CONTINUE, "allowed");
+ TEST_EQ(mock_ui_context.error_code, VB2_UI_ERROR_ALTFW_FAILED,
+ "ui_error code is set");
TEST_EQ(mock_run_altfw_called, 1, " vb2ex_run_altfw called once");
TEST_EQ(mock_altfw_last, 2, " select bootloader #2");
@@ -793,7 +801,9 @@
mock_ui_context.key = VB_KEY_CTRL('L');
mock_ui_context.state->selected_item = 4; /* Ignored */
TEST_EQ(vb2_ui_developer_mode_boot_altfw_action(&mock_ui_context),
- VB2_SUCCESS, "allowed: ctrl+l");
+ VB2_REQUEST_UI_CONTINUE, "allowed: ctrl+l");
+ TEST_EQ(mock_ui_context.error_code, VB2_UI_ERROR_ALTFW_FAILED,
+ "ui_error code is set");
TEST_EQ(mock_run_altfw_called, 1, " vb2ex_run_altfw called once");
TEST_EQ(mock_altfw_last, 0, " select bootloader #0");
