diff options
author | Joel Kitching <kitching@google.com> | 2021-02-24 14:19:44 +0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2021-03-05 07:58:03 +0000 |
commit | 1f2cd002b3c92c5f35b5becd95aedb820a8af569 (patch) | |
tree | db04c693cf7a8074fe9a5652a55299d9b36384d1 | |
parent | a893a93c04c37dae592c4fbfcb79d57f310890a3 (diff) | |
download | vboot-1f2cd002b3c92c5f35b5becd95aedb820a8af569.tar.gz |
vboot: always return after calling vb2_ui_screen_changestabilize-rust-13836.Bstabilize-13836.B
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 <kitching@google.com>
Change-Id: I820e387417ad39e2f7bd47f65d08c387cf66d6e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2717449
Tested-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
-rw-r--r-- | firmware/2lib/2ui_screens.c | 4 | ||||
-rw-r--r-- | 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); |