Re: [PATCH v13 5/9] bootmenu: add removable media entries

On Wed, 24 Aug 2022 at 23:17, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

Hi Kojima-san,

Apologies for doing the review ad-hoc, but the patchset is huge. I'll try to send a more complete email tomorrow

No problem, thank you for your review.

[...]

...

#endif

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 537f3f2bbc..92171c4894 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -1786,6 +1786,215 @@ static efi_status_t eficonfig_process_delete_boot_option(void *data) return ret; }

+/**

• • eficonfig_enumerate_boot_option() - enumerate the possible bootable media
• • @opt: pointer to the media boot option structure
• • @volume_handles: pointer to the efi handles
• • @count: number of efi handle
• • Return: status code
• */

+efi_status_t eficonfig_enumerate_boot_option(struct eficonfig_media_boot_option *opt,

•                                      efi_handle_t *volume_handles, efi_status_t count)
  

+{

• u32 i;
  

• struct efi_handler *handler;
  

• efi_status_t ret = EFI_SUCCESS;
  

• for (i = 0; i < count; i++) {
  

•         u16 *p;
  

•         u16 dev_name[BOOTMENU_DEVICE_NAME_MAX];
  

•         char *optional_data;
  

•         struct efi_load_option lo;
  

•         char buf[BOOTMENU_DEVICE_NAME_MAX];
  

•         struct efi_device_path *device_path;
  

•         ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
  

•         if (ret != EFI_SUCCESS)
  

•                 continue;
  

•         ret = efi_protocol_open(handler, (void **)&device_path,
  

•                                 efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
  

•         if (ret != EFI_SUCCESS)
  

•                 continue;
  

•         ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
  

•         if (ret != EFI_SUCCESS)
  

•                 continue;
  

•         p = dev_name;
  

•         utf8_utf16_strncpy(&p, buf, strlen(buf));
  

•         lo.label = dev_name;
  

•         lo.attributes = LOAD_OPTION_ACTIVE;
  

•         lo.file_path = device_path;
  

•         lo.file_path_length = efi_dp_size(device_path) + sizeof(END);
  

•         /*
  

•          * Set the dedicated guid to optional_data, it is used to identify
  

•          * the boot option that automatically generated by the bootmenu.
  

•          * efi_serialize_load_option() expects optional_data is null-terminated
  

•          * utf8 string, so set the "1234567" string to allocate enough space
  

•          * to store guid, instead of realloc the load_option.
  

•          */
  

•         lo.optional_data = "1234567";
  

•         opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo);
  

•         if (!opt[i].size) {
  

•                 ret = EFI_OUT_OF_RESOURCES;
  

•                 free(dev_name);
  

This is a stack allocated variable.

I will fix it.

Thanks, Masahisa Kojima

...

•                 goto out;
  

•         }
  

•         /* set the guid */
  

[...]

Thanks /Ilias