From 1f2cd002b3c92c5f35b5becd95aedb820a8af569 Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Wed, 24 Feb 2021 14:19:44 +0800 Subject: vboot: always return after calling vb2_ui_screen_change Without returning, subsequent code may operate under the assumption that the screen has *not* changed, leading to unexpected behaviour. The user may also be able to select otherwise disallowed menu items. BUG=b:181087237, chromium:1181484 TEST=make clean && make runtests BRANCH=none Signed-off-by: Joel Kitching Change-Id: I820e387417ad39e2f7bd47f65d08c387cf66d6e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2717449 Tested-by: Joel Kitching Reviewed-by: Yu-Ping Wu Commit-Queue: Joel Kitching --- firmware/2lib/2ui_screens.c | 4 ++-- firmware/2lib/include/2ui.h | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c index 3026f76e..adebc53e 100644 --- a/firmware/2lib/2ui_screens.c +++ b/firmware/2lib/2ui_screens.c @@ -690,7 +690,7 @@ vb2_error_t developer_mode_init(struct vb2_ui_context *ui) /* TODO(b/159579189): Split this case into a separate root screen */ if (!vb2_dev_boot_allowed(ui->ctx)) - vb2_ui_screen_change(ui, VB2_SCREEN_DEVELOPER_TO_NORM); + return vb2_ui_screen_change(ui, VB2_SCREEN_DEVELOPER_TO_NORM); /* Don't show "Return to secure mode" button if GBB forces dev mode. */ if (vb2_get_gbb(ui->ctx)->flags & VB2_GBB_FLAG_FORCE_DEV_SWITCH_ON) @@ -783,7 +783,7 @@ vb2_error_t developer_mode_action(struct vb2_ui_context *ui) /* TODO(b/159579189): Split this case into a separate root screen */ if (!vb2_dev_boot_allowed(ui->ctx)) - vb2_ui_screen_change(ui, VB2_SCREEN_DEVELOPER_TO_NORM); + return vb2_ui_screen_change(ui, VB2_SCREEN_DEVELOPER_TO_NORM); /* Once any user interaction occurs, stop the timer. */ if (ui->key) diff --git a/firmware/2lib/include/2ui.h b/firmware/2lib/include/2ui.h index c5d13311..c8e2928b 100644 --- a/firmware/2lib/include/2ui.h +++ b/firmware/2lib/include/2ui.h @@ -138,7 +138,7 @@ vb2_error_t vb2_ui_developer_mode_boot_altfw_action( /** * Get info struct of a screen. * - * @param screen Screen from enum vb2_screen + * @param id Screen from enum vb2_screen * * @return screen info struct on success, NULL on error. */ @@ -174,6 +174,9 @@ vb2_error_t vb2_ui_menu_next(struct vb2_ui_context *ui); /** * Select the current menu item. * + * The caller should take care of returning after this function, and should not + * continue executing under the assumption that the screen has *not* changed. + * * If the current menu item has an action associated with it, run the action. * Otherwise, navigate to the target screen. If neither of these are set, then * selecting the menu item is a no-op. @@ -189,7 +192,13 @@ vb2_error_t vb2_ui_menu_select(struct vb2_ui_context *ui); /** * Return back to the previous screen. * - * If the current screen is already the root scren, the request is ignored. + * The caller should take care of returning after this function, and should not + * continue executing under the assumption that the screen has *not* changed. + * + * If the current screen is already the root screen, the request is ignored. + * + * TODO(b/157625765): Consider falling into recovery mode (BROKEN screen) when + * the current screen is already the root screen. * * @param ui UI context pointer * @return VB2_REQUEST_UI_CONTINUE, or error code on error. @@ -199,9 +208,16 @@ vb2_error_t vb2_ui_screen_back(struct vb2_ui_context *ui); /** * Change to the given screen. * + * The caller should take care of returning after this function, and should not + * continue executing under the assumption that the screen has *not* changed. + * * If the screen is not found, the request is ignored. * + * TODO(b/157625765): Consider falling into recovery mode (BROKEN screen) when + * the target screen is not found. + * * @param ui UI context pointer + * @param id Screen from enum vb2_screen * @return VB2_REQUEST_UI_CONTINUE, or error code on error. */ vb2_error_t vb2_ui_screen_change(struct vb2_ui_context *ui, enum vb2_screen id); -- cgit v1.2.1