Re: [PATCH v5 11/17] bootmenu: add Kconfig option not to enter U-Boot console

2022-04-29 Thread Mark Kettenis
> From: Masahisa Kojima 
> Date: Thu, 28 Apr 2022 17:09:44 +0900
> 
> This commit adds the Kconfig option to disable to enter
> the U-Boot console from bootmenu.
> 
> If CMD_BOOTMENU_ENTER_UBOOT_CONSOLE is enabled, "U-Boot console"
> entry is appeared as the last entry in the bootmenu, then user can
> enter U-Boot console.
> 
> If CMD_BOOTMENU_ENTER_UBOOT_CONSOLE is disabled, "Quit" entry
> is appeared as the last entry instead of "U-Boot console".
> When user chooses "Quit" from bootmenu, the following default
> commands are invoked.
> 
>  - "bootefi bootmgr" (if efi bootmgr is enabled)
>  - "run bootcmd"
> 
> If the both commands are executed and returns to the bootmenu,
> the bootmenu will appears again.

I think the default for this option should be "y", otherwise I fear
too many boards will ship with a "locked down" U-Boot where the user
has no way to get at the U-Boot prompt.

> Signed-off-by: Masahisa Kojima 
> ---
> Changes in v5:
> - split into the separate patch
> - clear the console when user select "U-Boot console"
> - if the console is disabled, the last entry title is "Quit"
> 
>  cmd/Kconfig| 10 
>  cmd/bootmenu.c | 69 ++
>  2 files changed, 68 insertions(+), 11 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 2b575a2b42..99a1435467 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -356,6 +356,16 @@ config CMD_BOOTMENU
>   help
> Add an ANSI terminal boot menu command.
>  
> +config CMD_BOOTMENU_ENTER_UBOOT_CONSOLE
> + bool "Allow Bootmenu to enter the U-Boot console"
> + depends on CMD_BOOTMENU
> + default n
> + help
> +   Add an entry to enter U-Boot console in bootmenu.
> +   If this option is disabled, user can not enter
> +   the U-Boot console from bootmenu. It increases
> +   the system security.
> +
>  config CMD_ADTIMG
>   bool "adtimg"
>   help
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index afe42b8041..bfbb1b5248 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -29,6 +29,13 @@
>   */
>  #define MAX_ENV_SIZE (9 + 2 + 1)
>  
> +enum bootmenu_ret {
> + BOOTMENU_RET_SUCCESS = 0,
> + BOOTMENU_RET_FAIL,
> + BOOTMENU_RET_QUIT,
> + BOOTMENU_RET_UPDATED,
> +};
> +
>  enum boot_type {
>   BOOTMENU_TYPE_NONE = 0,
>   BOOTMENU_TYPE_BOOTMENU,
> @@ -681,7 +688,12 @@ static struct bootmenu_data *bootmenu_create(int delay)
>   if (!entry)
>   goto cleanup;
>  
> - entry->title = u16_strdup(u"U-Boot console");
> + /* Add Quit entry if entering U-Boot console is disabled */
> + if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE))
> + entry->title = u16_strdup(u"U-Boot console");
> + else
> + entry->title = u16_strdup(u"Quit");
> +
>   if (!entry->title) {
>   free(entry);
>   goto cleanup;
> @@ -777,15 +789,17 @@ static void handle_uefi_bootnext(void)
>   run_command("bootefi bootmgr", 0);
>  }
>  
> -static void bootmenu_show(int delay)
> +static enum bootmenu_ret bootmenu_show(int delay)
>  {
> + int cmd_ret;
>   int init = 0;
>   void *choice = NULL;
>   u16 *title = NULL;
>   char *command = NULL;
>   struct menu *menu;
> - struct bootmenu_data *bootmenu;
>   struct bootmenu_entry *iter;
> + int ret = BOOTMENU_RET_SUCCESS;
> + struct bootmenu_data *bootmenu;
>   efi_status_t efi_ret = EFI_SUCCESS;
>   char *option, *sep;
>  
> @@ -797,27 +811,27 @@ static void bootmenu_show(int delay)
>   option = bootmenu_getoption(0);
>   if (!option) {
>   puts("bootmenu option 0 was not found\n");
> - return;
> + return BOOTMENU_RET_FAIL;
>   }
>   sep = strchr(option, '=');
>   if (!sep) {
>   puts("bootmenu option 0 is invalid\n");
> - return;
> + return BOOTMENU_RET_FAIL;
>   }
> - run_command(sep+1, 0);
> - return;
> + cmd_ret = run_command(sep + 1, 0);
> + return (cmd_ret == CMD_RET_SUCCESS ? BOOTMENU_RET_SUCCESS : 
> BOOTMENU_RET_FAIL);
>   }
>  
>   bootmenu = bootmenu_create(delay);
>   if (!bootmenu)
> - return;
> + return BOOTMENU_RET_FAIL;
>  
>   menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
>  bootmenu_print_entry, bootmenu_choice_entry,
>  bootmenu);
>   if (!menu) {
>   bootmenu_destroy(bootmenu);
> - return;
> + return BOOTMENU_RET_FAIL;
>   }
>  
>   for (iter = bootmenu->first; iter; iter = iter->next) {
> @@ -838,6 +852,14 @@ static void bootmenu_show(int delay)
>   iter = choice;
>

[PATCH v5 11/17] bootmenu: add Kconfig option not to enter U-Boot console

2022-04-28 Thread Masahisa Kojima
This commit adds the Kconfig option to disable to enter
the U-Boot console from bootmenu.

If CMD_BOOTMENU_ENTER_UBOOT_CONSOLE is enabled, "U-Boot console"
entry is appeared as the last entry in the bootmenu, then user can
enter U-Boot console.

If CMD_BOOTMENU_ENTER_UBOOT_CONSOLE is disabled, "Quit" entry
is appeared as the last entry instead of "U-Boot console".
When user chooses "Quit" from bootmenu, the following default
commands are invoked.

 - "bootefi bootmgr" (if efi bootmgr is enabled)
 - "run bootcmd"

If the both commands are executed and returns to the bootmenu,
the bootmenu will appears again.

Signed-off-by: Masahisa Kojima 
---
Changes in v5:
- split into the separate patch
- clear the console when user select "U-Boot console"
- if the console is disabled, the last entry title is "Quit"

 cmd/Kconfig| 10 
 cmd/bootmenu.c | 69 ++
 2 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 2b575a2b42..99a1435467 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -356,6 +356,16 @@ config CMD_BOOTMENU
help
  Add an ANSI terminal boot menu command.
 
+config CMD_BOOTMENU_ENTER_UBOOT_CONSOLE
+   bool "Allow Bootmenu to enter the U-Boot console"
+   depends on CMD_BOOTMENU
+   default n
+   help
+ Add an entry to enter U-Boot console in bootmenu.
+ If this option is disabled, user can not enter
+ the U-Boot console from bootmenu. It increases
+ the system security.
+
 config CMD_ADTIMG
bool "adtimg"
help
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index afe42b8041..bfbb1b5248 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -29,6 +29,13 @@
  */
 #define MAX_ENV_SIZE   (9 + 2 + 1)
 
+enum bootmenu_ret {
+   BOOTMENU_RET_SUCCESS = 0,
+   BOOTMENU_RET_FAIL,
+   BOOTMENU_RET_QUIT,
+   BOOTMENU_RET_UPDATED,
+};
+
 enum boot_type {
BOOTMENU_TYPE_NONE = 0,
BOOTMENU_TYPE_BOOTMENU,
@@ -681,7 +688,12 @@ static struct bootmenu_data *bootmenu_create(int delay)
if (!entry)
goto cleanup;
 
-   entry->title = u16_strdup(u"U-Boot console");
+   /* Add Quit entry if entering U-Boot console is disabled */
+   if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE))
+   entry->title = u16_strdup(u"U-Boot console");
+   else
+   entry->title = u16_strdup(u"Quit");
+
if (!entry->title) {
free(entry);
goto cleanup;
@@ -777,15 +789,17 @@ static void handle_uefi_bootnext(void)
run_command("bootefi bootmgr", 0);
 }
 
-static void bootmenu_show(int delay)
+static enum bootmenu_ret bootmenu_show(int delay)
 {
+   int cmd_ret;
int init = 0;
void *choice = NULL;
u16 *title = NULL;
char *command = NULL;
struct menu *menu;
-   struct bootmenu_data *bootmenu;
struct bootmenu_entry *iter;
+   int ret = BOOTMENU_RET_SUCCESS;
+   struct bootmenu_data *bootmenu;
efi_status_t efi_ret = EFI_SUCCESS;
char *option, *sep;
 
@@ -797,27 +811,27 @@ static void bootmenu_show(int delay)
option = bootmenu_getoption(0);
if (!option) {
puts("bootmenu option 0 was not found\n");
-   return;
+   return BOOTMENU_RET_FAIL;
}
sep = strchr(option, '=');
if (!sep) {
puts("bootmenu option 0 is invalid\n");
-   return;
+   return BOOTMENU_RET_FAIL;
}
-   run_command(sep+1, 0);
-   return;
+   cmd_ret = run_command(sep + 1, 0);
+   return (cmd_ret == CMD_RET_SUCCESS ? BOOTMENU_RET_SUCCESS : 
BOOTMENU_RET_FAIL);
}
 
bootmenu = bootmenu_create(delay);
if (!bootmenu)
-   return;
+   return BOOTMENU_RET_FAIL;
 
menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
   bootmenu_print_entry, bootmenu_choice_entry,
   bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
-   return;
+   return BOOTMENU_RET_FAIL;
}
 
for (iter = bootmenu->first; iter; iter = iter->next) {
@@ -838,6 +852,14 @@ static void bootmenu_show(int delay)
iter = choice;
title = u16_strdup(iter->title);
command = strdup(iter->command);
+
+   /* last entry is U-Boot console or Quit */
+   if (iter->num == iter->menu->count - 1) {
+   ret = BOOTMENU_RET_QUIT;
+   goto cleanup;
+   }
+   } else {
+   goto cleanup;
}