vboot/ui: Refactor ui error handlingstabilize-RUST-13932.Bstabilize-13942.Bstabilize-13935.Bfactory-asurada-13929.Bfactory-13929.B

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>

3 files changed, 105 insertions, 75 deletions

diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c
index 1ac50edc..a9359b55 100644
--- a/firmware/2lib/2ui_screens.c
+++ b/firmware/2lib/2ui_screens.c
@@ -53,6 +53,32 @@ static vb2_error_t power_off_action(struct vb2_ui_context *ui)
 
 /******************************************************************************/
 /*
+ * 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 const struct vb2_screen_info advanced_options_screen = {
 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 @@ static vb2_error_t firmware_log_set_content(struct vb2_ui_context *ui,
 					    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 @@ static const struct vb2_screen_info recovery_invalid_screen = {
 
 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 @@ vb2_error_t recovery_to_dev_confirm_action(struct vb2_ui_context *ui)
 		 * 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_error_t vb2_ui_developer_mode_boot_external_action(
 	    !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 @@ static vb2_error_t developer_to_norm_init(struct vb2_ui_context *ui)
 	/* 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 const struct vb2_menu_item developer_select_bootloader_items_after[] = {
 
 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_error_t vb2_ui_developer_mode_boot_altfw_action(
 	    !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 @@ vb2_error_t vb2_ui_developer_mode_boot_altfw_action(
 	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 @@ static const struct vb2_screen_info diagnostics_screen = {
 #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_impl(
 
 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 @@ static vb2_error_t diagnostics_memory_update_screen(struct vb2_ui_context *ui,
 						    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 @@ static vb2_error_t diagnostics_memory_init_quick(struct vb2_ui_context *ui)
 {
 	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 f7acdc60..bd3b05f1 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 42546844..66a776eb 100644
--- a/tests/vb2_ui_action_tests.c
+++ b/tests/vb2_ui_action_tests.c
@@ -757,7 +757,9 @@ static void vb2_ui_developer_mode_boot_altfw_action_tests(void)
 	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 @@ static void vb2_ui_developer_mode_boot_altfw_action_tests(void)
 	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 @@ static void vb2_ui_developer_mode_boot_altfw_action_tests(void)
 	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 @@ static void vb2_ui_developer_mode_boot_altfw_action_tests(void)
 	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");