| Message ID | 20220428080950.23509-17-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | enable menu-driven boot device selection | expand |
On 4/28/22 10:09, Masahisa Kojima wrote: > UEFI specification requires booting from removal media using > a architecture-specific default image name such as BOOTAA64.EFI. > This commit adds the removable media entries into bootmenu, > so that user can select the removable media and boot with > default image. > > The bootmenu automatically enumerates the possible bootable > media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, > add it as new UEFI boot option(BOOT####) and update BootOrder > variable. This automatically generated UEFI boot option > has the dedicated guid in the optional_data to distinguish it from > the UEFI boot option user adds manually. > > This commit also provides the BOOT#### variable maintenance feature. > Depending on the system hardware setup, some devices > may not exist at a later system boot, so bootmenu checks the > available device in each bootmenu invocation and automatically > removes the BOOT#### variable corrensponding to the non-existent > media device. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > Changes in v5: > - Return EFI_SUCCESS if there is no BootOrder defined > - correctly handle the case if no removable device found > - use guid to identify the automatically generated entry by bootmenu > > Newly created in v4 > > cmd/bootmenu.c | 94 +++++++++++++++ > include/efi_loader.h | 20 ++++ > lib/efi_loader/efi_bootmenu_maintenance.c | 139 ++++++++++++++++++++++ > 3 files changed, 253 insertions(+) > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > index 860cb83182..970db3ee01 100644 > --- a/cmd/bootmenu.c > +++ b/cmd/bootmenu.c > @@ -396,6 +396,89 @@ static int is_blk_device_available(char *token) > return -ENODEV; > } > > +/** > + * prepare_media_device_entry() - generate the media device entries > + * > + * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL > + * and generate the bootmenu entries. > + * This function also provide the BOOT#### variable maintenance for > + * the media device entries. > + * - Automatically create the BOOT#### variable for the newly detected device, > + * this BOOT#### variable is distinguished by the special GUID > + * stored in the EFI_LOAD_OPTION.optional_data > + * - If the device is not attached to the system, the associated BOOT#### variable > + * is automatically deleted. > + * > + * Return: status code > + */ > +static efi_status_t prepare_media_device_entry(void) > +{ > + u32 i; > + efi_status_t ret; > + efi_uintn_t count; > + efi_handle_t *volume_handles = NULL; > + struct efi_bootmenu_media_boot_option *opt = NULL; > + > + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, > + NULL, &count, (efi_handle_t **)&volume_handles); > + if (ret != EFI_SUCCESS) > + return ret; > + > + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); > + if (!opt) > + goto out; > + > + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ > + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); > + if (ret != EFI_SUCCESS) > + goto out; > + > + /* > + * System hardware configuration may vary depending on the user setup. > + * The boot option is automatically added by the bootmenu. > + * If the device is not attached to the system, the boot option needs > + * to be deleted. > + */ > + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); > + if (ret != EFI_SUCCESS) > + goto out; > + > + /* add non-existent boot option */ > + for (i = 0; i < count; i++) { > + u32 boot_index; > + u16 var_name[9]; > + > + if (!opt[i].exist) { > + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), > + &boot_index); > + if (ret != EFI_SUCCESS) > + goto out; > + > + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, > + EFI_VARIABLE_NON_VOLATILE | > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS, > + opt[i].size, opt[i].lo, false); > + if (ret != EFI_SUCCESS) > + goto out; > + > + ret = efi_bootmenu_append_bootorder(boot_index); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + } > + > +out: > + if (opt) { > + for (i = 0; i < count; i++) > + free(opt[i].lo); > + } > + free(opt); > + efi_free_pool(volume_handles); > + > + return ret; > +} > + > /** > * prepare_distro_boot_entry() - generate the distro boot entries > * > @@ -500,6 +583,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, > static struct bootmenu_data *bootmenu_create(int delay) > { > int ret; > + efi_status_t efi_ret; > unsigned short int i = 0; > struct bootmenu_data *menu; > struct bootmenu_entry *iter = NULL; > @@ -523,6 +607,16 @@ static struct bootmenu_data *bootmenu_create(int delay) > goto cleanup; > > if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { > + if (i < MAX_DYNAMIC_ENTRY) { > + /* > + * UEFI specification requires booting from removal media using > + * a architecture-specific default image name such as BOOTAA64.EFI. > + */ > + efi_ret = prepare_media_device_entry(); > + if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) > + goto cleanup; > + } > + > if (i < MAX_DYNAMIC_ENTRY) { > ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > if (ret < 0 && ret != -ENOENT) > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 533618341b..bef492ccb9 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -928,6 +928,22 @@ struct efi_signature_store { > struct x509_certificate; > struct pkcs7_message; > > +/** > + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device > + * > + * This structure is used to enumerate possible boot option > + * > + * @lo: Serialized load option data > + * @size: Size of serialized load option data > + * @exist: Flag to indicate the load option already exists > + * in Non-volatile load option > + */ > +struct efi_bootmenu_media_boot_option { > + void *lo; > + efi_uintn_t size; > + bool exist; > +}; > + > bool efi_signature_lookup_digest(struct efi_image_regions *regs, > struct efi_signature_store *db, > bool dbx); > @@ -1076,6 +1092,10 @@ efi_status_t efi_console_get_u16_string > efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, > efi_uintn_t buf_size, u32 *index); > efi_status_t efi_bootmenu_append_bootorder(u16 index); > +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_media_boot_option *opt, > + efi_handle_t *volume_handles, efi_status_t count); > +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > + efi_status_t count); > > efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); > > diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c > index 8c3f94c695..33b37fd11a 100644 > --- a/lib/efi_loader/efi_bootmenu_maintenance.c > +++ b/lib/efi_loader/efi_bootmenu_maintenance.c > @@ -26,6 +26,13 @@ static struct efi_simple_text_output_protocol *cout; > #define EFI_BOOTMENU_BOOT_NAME_MAX 32 > #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 > > +#define EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID \ > + EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \ > + 0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde) > + > +static const efi_guid_t efi_guid_bootmenu_auto_generated = > + EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID; > + > typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); > > /** > @@ -1104,3 +1111,135 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) > ARRAY_SIZE(maintenance_menu_items), > -1); > } > + > +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_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++) { > + char *optional_data; > + u16 *dev_name, *p; > + struct efi_load_option lo; > + struct efi_block_io *block_io; > + 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_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); > + if (ret != EFI_SUCCESS) > + continue; > + ret = efi_protocol_open(handler, (void **)&block_io, > + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); > + if (ret != EFI_SUCCESS) > + continue; > + > + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); > + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); > + if (!dev_name) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > + } > + 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 "dummystr" string to allocate enough space > + * to store guid, instead of realloc the load_option. > + * > + * This will allocate 16 bytes for guid plus trailing 0x0000. > + * The guid does not require trailing 0x0000, but it is for safety > + * in case some program handle the optional_data as u16 string. > + */ > + lo.optional_data = "dummystr"; Why do you want to reserve 18 bytes when 16 are enough for the GUID? > + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); > + if (!opt[i].size) { > + ret = EFI_OUT_OF_RESOURCES; > + free(dev_name); > + goto out; > + } > + /* set the guid */ > + optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"dummystr")); > + memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); > + free(dev_name); > + } > + > +out: > + return ret; > +} > + > +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > + efi_status_t count) > +{ > + u16 *bootorder; > + u32 i, j; > + efi_status_t ret; > + efi_uintn_t num, size, bootorder_size; > + void *load_option; > + struct efi_load_option lo; > + u16 varname[] = u"Boot####"; > + > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); > + if (!bootorder) > + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ > + > + num = bootorder_size / sizeof(u16); > + for (i = 0; i < num;) { > + efi_uintn_t tmp; > + > + efi_create_indexed_name(varname, sizeof(varname), > + "Boot", bootorder[i]); > + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); > + if (!load_option) > + goto next; > + > + tmp = size; This copying of variables is superfluous. Just keep size. > + ret = efi_deserialize_load_option(&lo, load_option, &tmp); > + if (ret != EFI_SUCCESS) > + goto next; > + > + if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { You must avoid a buffer overrun. Check size >= 16. Best regards Heinrich > + for (j = 0; j < count; j++) { > + if (memcmp(opt[j].lo, load_option, size) == 0) { > + opt[j].exist = true; > + break; > + } > + } > + > + if (j == count) { > + ret = delete_boot_option(bootorder, i, bootorder_size); > + if (ret != EFI_SUCCESS) { > + free(load_option); > + goto out; > + } > + > + num--; > + bootorder_size -= sizeof(u16); > + free(load_option); > + continue; > + } > + } > +next: > + free(load_option); > + i++; > + } > + > +out: > + return ret; > +}
On Fri, 29 Apr 2022 at 01:53, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 4/28/22 10:09, Masahisa Kojima wrote: > > UEFI specification requires booting from removal media using > > a architecture-specific default image name such as BOOTAA64.EFI. > > This commit adds the removable media entries into bootmenu, > > so that user can select the removable media and boot with > > default image. > > > > The bootmenu automatically enumerates the possible bootable > > media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, > > add it as new UEFI boot option(BOOT####) and update BootOrder > > variable. This automatically generated UEFI boot option > > has the dedicated guid in the optional_data to distinguish it from > > the UEFI boot option user adds manually. > > > > This commit also provides the BOOT#### variable maintenance feature. > > Depending on the system hardware setup, some devices > > may not exist at a later system boot, so bootmenu checks the > > available device in each bootmenu invocation and automatically > > removes the BOOT#### variable corrensponding to the non-existent > > media device. > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > Changes in v5: > > - Return EFI_SUCCESS if there is no BootOrder defined > > - correctly handle the case if no removable device found > > - use guid to identify the automatically generated entry by bootmenu > > > > Newly created in v4 > > > > cmd/bootmenu.c | 94 +++++++++++++++ > > include/efi_loader.h | 20 ++++ > > lib/efi_loader/efi_bootmenu_maintenance.c | 139 ++++++++++++++++++++++ > > 3 files changed, 253 insertions(+) > > > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > > index 860cb83182..970db3ee01 100644 > > --- a/cmd/bootmenu.c > > +++ b/cmd/bootmenu.c > > @@ -396,6 +396,89 @@ static int is_blk_device_available(char *token) > > return -ENODEV; > > } > > > > +/** > > + * prepare_media_device_entry() - generate the media device entries > > + * > > + * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL > > + * and generate the bootmenu entries. > > + * This function also provide the BOOT#### variable maintenance for > > + * the media device entries. > > + * - Automatically create the BOOT#### variable for the newly detected device, > > + * this BOOT#### variable is distinguished by the special GUID > > + * stored in the EFI_LOAD_OPTION.optional_data > > + * - If the device is not attached to the system, the associated BOOT#### variable > > + * is automatically deleted. > > + * > > + * Return: status code > > + */ > > +static efi_status_t prepare_media_device_entry(void) > > +{ > > + u32 i; > > + efi_status_t ret; > > + efi_uintn_t count; > > + efi_handle_t *volume_handles = NULL; > > + struct efi_bootmenu_media_boot_option *opt = NULL; > > + > > + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, > > + NULL, &count, (efi_handle_t **)&volume_handles); > > + if (ret != EFI_SUCCESS) > > + return ret; > > + > > + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); > > + if (!opt) > > + goto out; > > + > > + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ > > + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + /* > > + * System hardware configuration may vary depending on the user setup. > > + * The boot option is automatically added by the bootmenu. > > + * If the device is not attached to the system, the boot option needs > > + * to be deleted. > > + */ > > + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + /* add non-existent boot option */ > > + for (i = 0; i < count; i++) { > > + u32 boot_index; > > + u16 var_name[9]; > > + > > + if (!opt[i].exist) { > > + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), > > + &boot_index); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, > > + EFI_VARIABLE_NON_VOLATILE | > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS, > > + opt[i].size, opt[i].lo, false); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + ret = efi_bootmenu_append_bootorder(boot_index); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + } > > + } > > + > > +out: > > + if (opt) { > > + for (i = 0; i < count; i++) > > + free(opt[i].lo); > > + } > > + free(opt); > > + efi_free_pool(volume_handles); > > + > > + return ret; > > +} > > + > > /** > > * prepare_distro_boot_entry() - generate the distro boot entries > > * > > @@ -500,6 +583,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, > > static struct bootmenu_data *bootmenu_create(int delay) > > { > > int ret; > > + efi_status_t efi_ret; > > unsigned short int i = 0; > > struct bootmenu_data *menu; > > struct bootmenu_entry *iter = NULL; > > @@ -523,6 +607,16 @@ static struct bootmenu_data *bootmenu_create(int delay) > > goto cleanup; > > > > if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { > > + if (i < MAX_DYNAMIC_ENTRY) { > > + /* > > + * UEFI specification requires booting from removal media using > > + * a architecture-specific default image name such as BOOTAA64.EFI. > > + */ > > + efi_ret = prepare_media_device_entry(); > > + if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) > > + goto cleanup; > > + } > > + > > if (i < MAX_DYNAMIC_ENTRY) { > > ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > > if (ret < 0 && ret != -ENOENT) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 533618341b..bef492ccb9 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -928,6 +928,22 @@ struct efi_signature_store { > > struct x509_certificate; > > struct pkcs7_message; > > > > +/** > > + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device > > + * > > + * This structure is used to enumerate possible boot option > > + * > > + * @lo: Serialized load option data > > + * @size: Size of serialized load option data > > + * @exist: Flag to indicate the load option already exists > > + * in Non-volatile load option > > + */ > > +struct efi_bootmenu_media_boot_option { > > + void *lo; > > + efi_uintn_t size; > > + bool exist; > > +}; > > + > > bool efi_signature_lookup_digest(struct efi_image_regions *regs, > > struct efi_signature_store *db, > > bool dbx); > > @@ -1076,6 +1092,10 @@ efi_status_t efi_console_get_u16_string > > efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, > > efi_uintn_t buf_size, u32 *index); > > efi_status_t efi_bootmenu_append_bootorder(u16 index); > > +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_media_boot_option *opt, > > + efi_handle_t *volume_handles, efi_status_t count); > > +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > > + efi_status_t count); > > > > efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); > > > > diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c > > index 8c3f94c695..33b37fd11a 100644 > > --- a/lib/efi_loader/efi_bootmenu_maintenance.c > > +++ b/lib/efi_loader/efi_bootmenu_maintenance.c > > @@ -26,6 +26,13 @@ static struct efi_simple_text_output_protocol *cout; > > #define EFI_BOOTMENU_BOOT_NAME_MAX 32 > > #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 > > > > +#define EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID \ > > + EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \ > > + 0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde) > > + > > +static const efi_guid_t efi_guid_bootmenu_auto_generated = > > + EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID; > > + > > typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); > > > > /** > > @@ -1104,3 +1111,135 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) > > ARRAY_SIZE(maintenance_menu_items), > > -1); > > } > > + > > +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_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++) { > > + char *optional_data; > > + u16 *dev_name, *p; > > + struct efi_load_option lo; > > + struct efi_block_io *block_io; > > + 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_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); > > + if (ret != EFI_SUCCESS) > > + continue; > > + ret = efi_protocol_open(handler, (void **)&block_io, > > + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); > > + if (ret != EFI_SUCCESS) > > + continue; > > + > > + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); > > + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); > > + if (!dev_name) { > > + ret = EFI_OUT_OF_RESOURCES; > > + goto out; > > + } > > + 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 "dummystr" string to allocate enough space > > + * to store guid, instead of realloc the load_option. > > + * > > + * This will allocate 16 bytes for guid plus trailing 0x0000. > > + * The guid does not require trailing 0x0000, but it is for safety > > + * in case some program handle the optional_data as u16 string. > > + */ > > + lo.optional_data = "dummystr"; > > Why do you want to reserve 18 bytes when 16 are enough for the GUID? OK, I will only allocate 16 bytes for GUID. > > > + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); > > + if (!opt[i].size) { > > + ret = EFI_OUT_OF_RESOURCES; > > + free(dev_name); > > + goto out; > > + } > > + /* set the guid */ > > + optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"dummystr")); > > + memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); > > + free(dev_name); > > + } > > + > > +out: > > + return ret; > > +} > > + > > +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > > + efi_status_t count) > > +{ > > + u16 *bootorder; > > + u32 i, j; > > + efi_status_t ret; > > + efi_uintn_t num, size, bootorder_size; > > + void *load_option; > > + struct efi_load_option lo; > > + u16 varname[] = u"Boot####"; > > + > > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); > > + if (!bootorder) > > + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ > > + > > + num = bootorder_size / sizeof(u16); > > + for (i = 0; i < num;) { > > + efi_uintn_t tmp; > > + > > + efi_create_indexed_name(varname, sizeof(varname), > > + "Boot", bootorder[i]); > > + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); > > + if (!load_option) > > + goto next; > > + > > + tmp = size; > > This copying of variables is superfluous. Just keep size. I don't catch your point. This efi_bootmenu_delete_invalid_boot_option() function does not copy the variable, only keeps the variable size. > > > + ret = efi_deserialize_load_option(&lo, load_option, &tmp); > > + if (ret != EFI_SUCCESS) > > + goto next; > > + > > + if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { > > You must avoid a buffer overrun. Check size >= 16. Yes, I need to check the size of optional_data. Thanks, Masahisa Kojima > > Best regards > > Heinrich > > > + for (j = 0; j < count; j++) { > > + if (memcmp(opt[j].lo, load_option, size) == 0) { > > + opt[j].exist = true; > > + break; > > + } > > + } > > + > > + if (j == count) { > > + ret = delete_boot_option(bootorder, i, bootorder_size); > > + if (ret != EFI_SUCCESS) { > > + free(load_option); > > + goto out; > > + } > > + > > + num--; > > + bootorder_size -= sizeof(u16); > > + free(load_option); > > + continue; > > + } > > + } > > +next: > > + free(load_option); > > + i++; > > + } > > + > > +out: > > + return ret; > > +} >
On 5/9/22 10:23, Masahisa Kojima wrote: > On Fri, 29 Apr 2022 at 01:53, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 4/28/22 10:09, Masahisa Kojima wrote: >>> UEFI specification requires booting from removal media using >>> a architecture-specific default image name such as BOOTAA64.EFI. >>> This commit adds the removable media entries into bootmenu, >>> so that user can select the removable media and boot with >>> default image. >>> >>> The bootmenu automatically enumerates the possible bootable >>> media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, >>> add it as new UEFI boot option(BOOT####) and update BootOrder >>> variable. This automatically generated UEFI boot option >>> has the dedicated guid in the optional_data to distinguish it from >>> the UEFI boot option user adds manually. >>> >>> This commit also provides the BOOT#### variable maintenance feature. >>> Depending on the system hardware setup, some devices >>> may not exist at a later system boot, so bootmenu checks the >>> available device in each bootmenu invocation and automatically >>> removes the BOOT#### variable corrensponding to the non-existent >>> media device. >>> >>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> >>> --- >>> Changes in v5: >>> - Return EFI_SUCCESS if there is no BootOrder defined >>> - correctly handle the case if no removable device found >>> - use guid to identify the automatically generated entry by bootmenu >>> >>> Newly created in v4 >>> >>> cmd/bootmenu.c | 94 +++++++++++++++ >>> include/efi_loader.h | 20 ++++ >>> lib/efi_loader/efi_bootmenu_maintenance.c | 139 ++++++++++++++++++++++ >>> 3 files changed, 253 insertions(+) >>> >>> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c >>> index 860cb83182..970db3ee01 100644 >>> --- a/cmd/bootmenu.c >>> +++ b/cmd/bootmenu.c >>> @@ -396,6 +396,89 @@ static int is_blk_device_available(char *token) >>> return -ENODEV; >>> } >>> >>> +/** >>> + * prepare_media_device_entry() - generate the media device entries >>> + * >>> + * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL >>> + * and generate the bootmenu entries. >>> + * This function also provide the BOOT#### variable maintenance for >>> + * the media device entries. >>> + * - Automatically create the BOOT#### variable for the newly detected device, >>> + * this BOOT#### variable is distinguished by the special GUID >>> + * stored in the EFI_LOAD_OPTION.optional_data >>> + * - If the device is not attached to the system, the associated BOOT#### variable >>> + * is automatically deleted. >>> + * >>> + * Return: status code >>> + */ >>> +static efi_status_t prepare_media_device_entry(void) >>> +{ >>> + u32 i; >>> + efi_status_t ret; >>> + efi_uintn_t count; >>> + efi_handle_t *volume_handles = NULL; >>> + struct efi_bootmenu_media_boot_option *opt = NULL; >>> + >>> + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, >>> + NULL, &count, (efi_handle_t **)&volume_handles); >>> + if (ret != EFI_SUCCESS) >>> + return ret; >>> + >>> + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); >>> + if (!opt) >>> + goto out; >>> + >>> + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ >>> + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + /* >>> + * System hardware configuration may vary depending on the user setup. >>> + * The boot option is automatically added by the bootmenu. >>> + * If the device is not attached to the system, the boot option needs >>> + * to be deleted. >>> + */ >>> + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + /* add non-existent boot option */ >>> + for (i = 0; i < count; i++) { >>> + u32 boot_index; >>> + u16 var_name[9]; >>> + >>> + if (!opt[i].exist) { >>> + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), >>> + &boot_index); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, >>> + EFI_VARIABLE_NON_VOLATILE | >>> + EFI_VARIABLE_BOOTSERVICE_ACCESS | >>> + EFI_VARIABLE_RUNTIME_ACCESS, >>> + opt[i].size, opt[i].lo, false); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + ret = efi_bootmenu_append_bootorder(boot_index); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + } >>> + } >>> + >>> +out: >>> + if (opt) { >>> + for (i = 0; i < count; i++) >>> + free(opt[i].lo); >>> + } >>> + free(opt); >>> + efi_free_pool(volume_handles); >>> + >>> + return ret; >>> +} >>> + >>> /** >>> * prepare_distro_boot_entry() - generate the distro boot entries >>> * >>> @@ -500,6 +583,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, >>> static struct bootmenu_data *bootmenu_create(int delay) >>> { >>> int ret; >>> + efi_status_t efi_ret; >>> unsigned short int i = 0; >>> struct bootmenu_data *menu; >>> struct bootmenu_entry *iter = NULL; >>> @@ -523,6 +607,16 @@ static struct bootmenu_data *bootmenu_create(int delay) >>> goto cleanup; >>> >>> if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { >>> + if (i < MAX_DYNAMIC_ENTRY) { >>> + /* >>> + * UEFI specification requires booting from removal media using >>> + * a architecture-specific default image name such as BOOTAA64.EFI. >>> + */ >>> + efi_ret = prepare_media_device_entry(); >>> + if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) >>> + goto cleanup; >>> + } >>> + >>> if (i < MAX_DYNAMIC_ENTRY) { >>> ret = prepare_uefi_bootorder_entry(menu, &iter, &i); >>> if (ret < 0 && ret != -ENOENT) >>> diff --git a/include/efi_loader.h b/include/efi_loader.h >>> index 533618341b..bef492ccb9 100644 >>> --- a/include/efi_loader.h >>> +++ b/include/efi_loader.h >>> @@ -928,6 +928,22 @@ struct efi_signature_store { >>> struct x509_certificate; >>> struct pkcs7_message; >>> >>> +/** >>> + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device >>> + * >>> + * This structure is used to enumerate possible boot option >>> + * >>> + * @lo: Serialized load option data >>> + * @size: Size of serialized load option data >>> + * @exist: Flag to indicate the load option already exists >>> + * in Non-volatile load option >>> + */ >>> +struct efi_bootmenu_media_boot_option { >>> + void *lo; >>> + efi_uintn_t size; >>> + bool exist; >>> +}; >>> + >>> bool efi_signature_lookup_digest(struct efi_image_regions *regs, >>> struct efi_signature_store *db, >>> bool dbx); >>> @@ -1076,6 +1092,10 @@ efi_status_t efi_console_get_u16_string >>> efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, >>> efi_uintn_t buf_size, u32 *index); >>> efi_status_t efi_bootmenu_append_bootorder(u16 index); >>> +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_media_boot_option *opt, >>> + efi_handle_t *volume_handles, efi_status_t count); >>> +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, >>> + efi_status_t count); >>> >>> efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); >>> >>> diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c >>> index 8c3f94c695..33b37fd11a 100644 >>> --- a/lib/efi_loader/efi_bootmenu_maintenance.c >>> +++ b/lib/efi_loader/efi_bootmenu_maintenance.c >>> @@ -26,6 +26,13 @@ static struct efi_simple_text_output_protocol *cout; >>> #define EFI_BOOTMENU_BOOT_NAME_MAX 32 >>> #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 >>> >>> +#define EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID \ >>> + EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \ >>> + 0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde) >>> + >>> +static const efi_guid_t efi_guid_bootmenu_auto_generated = >>> + EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID; >>> + >>> typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); >>> >>> /** >>> @@ -1104,3 +1111,135 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) >>> ARRAY_SIZE(maintenance_menu_items), >>> -1); >>> } >>> + >>> +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_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++) { >>> + char *optional_data; >>> + u16 *dev_name, *p; >>> + struct efi_load_option lo; >>> + struct efi_block_io *block_io; >>> + 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_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); >>> + if (ret != EFI_SUCCESS) >>> + continue; >>> + ret = efi_protocol_open(handler, (void **)&block_io, >>> + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); >>> + if (ret != EFI_SUCCESS) >>> + continue; >>> + >>> + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); >>> + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); >>> + if (!dev_name) { >>> + ret = EFI_OUT_OF_RESOURCES; >>> + goto out; >>> + } >>> + 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 "dummystr" string to allocate enough space >>> + * to store guid, instead of realloc the load_option. >>> + * >>> + * This will allocate 16 bytes for guid plus trailing 0x0000. >>> + * The guid does not require trailing 0x0000, but it is for safety >>> + * in case some program handle the optional_data as u16 string. >>> + */ >>> + lo.optional_data = "dummystr"; >> >> Why do you want to reserve 18 bytes when 16 are enough for the GUID? > > OK, I will only allocate 16 bytes for GUID. > >> >>> + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); >>> + if (!opt[i].size) { >>> + ret = EFI_OUT_OF_RESOURCES; >>> + free(dev_name); >>> + goto out; >>> + } >>> + /* set the guid */ >>> + optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"dummystr")); >>> + memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); >>> + free(dev_name); >>> + } >>> + >>> +out: >>> + return ret; >>> +} >>> + >>> +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, >>> + efi_status_t count) >>> +{ >>> + u16 *bootorder; >>> + u32 i, j; >>> + efi_status_t ret; >>> + efi_uintn_t num, size, bootorder_size; >>> + void *load_option; >>> + struct efi_load_option lo; >>> + u16 varname[] = u"Boot####"; >>> + >>> + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); >>> + if (!bootorder) >>> + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ >>> + >>> + num = bootorder_size / sizeof(u16); >>> + for (i = 0; i < num;) { >>> + efi_uintn_t tmp; >>> + >>> + efi_create_indexed_name(varname, sizeof(varname), >>> + "Boot", bootorder[i]); >>> + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); >>> + if (!load_option) >>> + goto next; >>> + >>> + tmp = size; >> >> This copying of variables is superfluous. Just keep size. > > I don't catch your point. > This efi_bootmenu_delete_invalid_boot_option() function > does not copy the variable, only keeps the variable size. There is no need to copy the value of variable size to a new variable tmp. You can remove the variable tmp and continue using variable size below as both variables are of the same type. Best regards Heinrich > >> >>> + ret = efi_deserialize_load_option(&lo, load_option, &tmp); >>> + if (ret != EFI_SUCCESS) >>> + goto next; >>> + >>> + if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { >> >> You must avoid a buffer overrun. Check size >= 16. > > Yes, I need to check the size of optional_data. > > Thanks, > Masahisa Kojima > >> >> Best regards >> >> Heinrich >> >>> + for (j = 0; j < count; j++) { >>> + if (memcmp(opt[j].lo, load_option, size) == 0) { >>> + opt[j].exist = true; >>> + break; >>> + } >>> + } >>> + >>> + if (j == count) { >>> + ret = delete_boot_option(bootorder, i, bootorder_size); >>> + if (ret != EFI_SUCCESS) { >>> + free(load_option); >>> + goto out; >>> + } >>> + >>> + num--; >>> + bootorder_size -= sizeof(u16); >>> + free(load_option); >>> + continue; >>> + } >>> + } >>> +next: >>> + free(load_option); >>> + i++; >>> + } >>> + >>> +out: >>> + return ret; >>> +} >>
Hi Heinrich, On Mon, 9 May 2022 at 22:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 5/9/22 10:23, Masahisa Kojima wrote: > > On Fri, 29 Apr 2022 at 01:53, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 4/28/22 10:09, Masahisa Kojima wrote: > >>> UEFI specification requires booting from removal media using > >>> a architecture-specific default image name such as BOOTAA64.EFI. > >>> This commit adds the removable media entries into bootmenu, > >>> so that user can select the removable media and boot with > >>> default image. > >>> > >>> The bootmenu automatically enumerates the possible bootable > >>> media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, > >>> add it as new UEFI boot option(BOOT####) and update BootOrder > >>> variable. This automatically generated UEFI boot option > >>> has the dedicated guid in the optional_data to distinguish it from > >>> the UEFI boot option user adds manually. > >>> > >>> This commit also provides the BOOT#### variable maintenance feature. > >>> Depending on the system hardware setup, some devices > >>> may not exist at a later system boot, so bootmenu checks the > >>> available device in each bootmenu invocation and automatically > >>> removes the BOOT#### variable corrensponding to the non-existent > >>> media device. > >>> > >>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > >>> --- > >>> Changes in v5: > >>> - Return EFI_SUCCESS if there is no BootOrder defined > >>> - correctly handle the case if no removable device found > >>> - use guid to identify the automatically generated entry by bootmenu > >>> > >>> Newly created in v4 > >>> > >>> cmd/bootmenu.c | 94 +++++++++++++++ > >>> include/efi_loader.h | 20 ++++ > >>> lib/efi_loader/efi_bootmenu_maintenance.c | 139 ++++++++++++++++++++++ > >>> 3 files changed, 253 insertions(+) > >>> > >>> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > >>> index 860cb83182..970db3ee01 100644 > >>> --- a/cmd/bootmenu.c > >>> +++ b/cmd/bootmenu.c > >>> @@ -396,6 +396,89 @@ static int is_blk_device_available(char *token) > >>> return -ENODEV; > >>> } > >>> > >>> +/** > >>> + * prepare_media_device_entry() - generate the media device entries > >>> + * > >>> + * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL > >>> + * and generate the bootmenu entries. > >>> + * This function also provide the BOOT#### variable maintenance for > >>> + * the media device entries. > >>> + * - Automatically create the BOOT#### variable for the newly detected device, > >>> + * this BOOT#### variable is distinguished by the special GUID > >>> + * stored in the EFI_LOAD_OPTION.optional_data > >>> + * - If the device is not attached to the system, the associated BOOT#### variable > >>> + * is automatically deleted. > >>> + * > >>> + * Return: status code > >>> + */ > >>> +static efi_status_t prepare_media_device_entry(void) > >>> +{ > >>> + u32 i; > >>> + efi_status_t ret; > >>> + efi_uintn_t count; > >>> + efi_handle_t *volume_handles = NULL; > >>> + struct efi_bootmenu_media_boot_option *opt = NULL; > >>> + > >>> + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, > >>> + NULL, &count, (efi_handle_t **)&volume_handles); > >>> + if (ret != EFI_SUCCESS) > >>> + return ret; > >>> + > >>> + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); > >>> + if (!opt) > >>> + goto out; > >>> + > >>> + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ > >>> + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + /* > >>> + * System hardware configuration may vary depending on the user setup. > >>> + * The boot option is automatically added by the bootmenu. > >>> + * If the device is not attached to the system, the boot option needs > >>> + * to be deleted. > >>> + */ > >>> + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + /* add non-existent boot option */ > >>> + for (i = 0; i < count; i++) { > >>> + u32 boot_index; > >>> + u16 var_name[9]; > >>> + > >>> + if (!opt[i].exist) { > >>> + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), > >>> + &boot_index); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, > >>> + EFI_VARIABLE_NON_VOLATILE | > >>> + EFI_VARIABLE_BOOTSERVICE_ACCESS | > >>> + EFI_VARIABLE_RUNTIME_ACCESS, > >>> + opt[i].size, opt[i].lo, false); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + ret = efi_bootmenu_append_bootorder(boot_index); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + } > >>> + } > >>> + > >>> +out: > >>> + if (opt) { > >>> + for (i = 0; i < count; i++) > >>> + free(opt[i].lo); > >>> + } > >>> + free(opt); > >>> + efi_free_pool(volume_handles); > >>> + > >>> + return ret; > >>> +} > >>> + > >>> /** > >>> * prepare_distro_boot_entry() - generate the distro boot entries > >>> * > >>> @@ -500,6 +583,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, > >>> static struct bootmenu_data *bootmenu_create(int delay) > >>> { > >>> int ret; > >>> + efi_status_t efi_ret; > >>> unsigned short int i = 0; > >>> struct bootmenu_data *menu; > >>> struct bootmenu_entry *iter = NULL; > >>> @@ -523,6 +607,16 @@ static struct bootmenu_data *bootmenu_create(int delay) > >>> goto cleanup; > >>> > >>> if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { > >>> + if (i < MAX_DYNAMIC_ENTRY) { > >>> + /* > >>> + * UEFI specification requires booting from removal media using > >>> + * a architecture-specific default image name such as BOOTAA64.EFI. > >>> + */ > >>> + efi_ret = prepare_media_device_entry(); > >>> + if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) > >>> + goto cleanup; > >>> + } > >>> + > >>> if (i < MAX_DYNAMIC_ENTRY) { > >>> ret = prepare_uefi_bootorder_entry(menu, &iter, &i); > >>> if (ret < 0 && ret != -ENOENT) > >>> diff --git a/include/efi_loader.h b/include/efi_loader.h > >>> index 533618341b..bef492ccb9 100644 > >>> --- a/include/efi_loader.h > >>> +++ b/include/efi_loader.h > >>> @@ -928,6 +928,22 @@ struct efi_signature_store { > >>> struct x509_certificate; > >>> struct pkcs7_message; > >>> > >>> +/** > >>> + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device > >>> + * > >>> + * This structure is used to enumerate possible boot option > >>> + * > >>> + * @lo: Serialized load option data > >>> + * @size: Size of serialized load option data > >>> + * @exist: Flag to indicate the load option already exists > >>> + * in Non-volatile load option > >>> + */ > >>> +struct efi_bootmenu_media_boot_option { > >>> + void *lo; > >>> + efi_uintn_t size; > >>> + bool exist; > >>> +}; > >>> + > >>> bool efi_signature_lookup_digest(struct efi_image_regions *regs, > >>> struct efi_signature_store *db, > >>> bool dbx); > >>> @@ -1076,6 +1092,10 @@ efi_status_t efi_console_get_u16_string > >>> efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, > >>> efi_uintn_t buf_size, u32 *index); > >>> efi_status_t efi_bootmenu_append_bootorder(u16 index); > >>> +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_media_boot_option *opt, > >>> + efi_handle_t *volume_handles, efi_status_t count); > >>> +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > >>> + efi_status_t count); > >>> > >>> efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); > >>> > >>> diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c > >>> index 8c3f94c695..33b37fd11a 100644 > >>> --- a/lib/efi_loader/efi_bootmenu_maintenance.c > >>> +++ b/lib/efi_loader/efi_bootmenu_maintenance.c > >>> @@ -26,6 +26,13 @@ static struct efi_simple_text_output_protocol *cout; > >>> #define EFI_BOOTMENU_BOOT_NAME_MAX 32 > >>> #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 > >>> > >>> +#define EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID \ > >>> + EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \ > >>> + 0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde) > >>> + > >>> +static const efi_guid_t efi_guid_bootmenu_auto_generated = > >>> + EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID; > >>> + > >>> typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); > >>> > >>> /** > >>> @@ -1104,3 +1111,135 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) > >>> ARRAY_SIZE(maintenance_menu_items), > >>> -1); > >>> } > >>> + > >>> +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_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++) { > >>> + char *optional_data; > >>> + u16 *dev_name, *p; > >>> + struct efi_load_option lo; > >>> + struct efi_block_io *block_io; > >>> + 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_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); > >>> + if (ret != EFI_SUCCESS) > >>> + continue; > >>> + ret = efi_protocol_open(handler, (void **)&block_io, > >>> + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); > >>> + if (ret != EFI_SUCCESS) > >>> + continue; > >>> + > >>> + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); > >>> + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); > >>> + if (!dev_name) { > >>> + ret = EFI_OUT_OF_RESOURCES; > >>> + goto out; > >>> + } > >>> + 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 "dummystr" string to allocate enough space > >>> + * to store guid, instead of realloc the load_option. > >>> + * > >>> + * This will allocate 16 bytes for guid plus trailing 0x0000. > >>> + * The guid does not require trailing 0x0000, but it is for safety > >>> + * in case some program handle the optional_data as u16 string. > >>> + */ > >>> + lo.optional_data = "dummystr"; > >> > >> Why do you want to reserve 18 bytes when 16 are enough for the GUID? > > > > OK, I will only allocate 16 bytes for GUID. > > > >> > >>> + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); > >>> + if (!opt[i].size) { > >>> + ret = EFI_OUT_OF_RESOURCES; > >>> + free(dev_name); > >>> + goto out; > >>> + } > >>> + /* set the guid */ > >>> + optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"dummystr")); > >>> + memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); > >>> + free(dev_name); > >>> + } > >>> + > >>> +out: > >>> + return ret; > >>> +} > >>> + > >>> +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, > >>> + efi_status_t count) > >>> +{ > >>> + u16 *bootorder; > >>> + u32 i, j; > >>> + efi_status_t ret; > >>> + efi_uintn_t num, size, bootorder_size; > >>> + void *load_option; > >>> + struct efi_load_option lo; > >>> + u16 varname[] = u"Boot####"; > >>> + > >>> + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); > >>> + if (!bootorder) > >>> + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ > >>> + > >>> + num = bootorder_size / sizeof(u16); > >>> + for (i = 0; i < num;) { > >>> + efi_uintn_t tmp; > >>> + > >>> + efi_create_indexed_name(varname, sizeof(varname), > >>> + "Boot", bootorder[i]); > >>> + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); > >>> + if (!load_option) > >>> + goto next; > >>> + > >>> + tmp = size; > >> > >> This copying of variables is superfluous. Just keep size. > > > > I don't catch your point. > > This efi_bootmenu_delete_invalid_boot_option() function > > does not copy the variable, only keeps the variable size. > > There is no need to copy the value of variable size to a new variable > tmp. You can remove the variable tmp and continue using variable size > below as both variables are of the same type. tmp is used for whole efi_load_option structure comparison, so I need to save the original size. Thanks, Masahisa Kojima > > Best regards > > Heinrich > > > > >> > >>> + ret = efi_deserialize_load_option(&lo, load_option, &tmp); > >>> + if (ret != EFI_SUCCESS) > >>> + goto next; > >>> + > >>> + if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { > >> > >> You must avoid a buffer overrun. Check size >= 16. > > > > Yes, I need to check the size of optional_data. > > > > Thanks, > > Masahisa Kojima > > > >> > >> Best regards > >> > >> Heinrich > >> > >>> + for (j = 0; j < count; j++) { > >>> + if (memcmp(opt[j].lo, load_option, size) == 0) { > >>> + opt[j].exist = true; > >>> + break; > >>> + } > >>> + } > >>> + > >>> + if (j == count) { > >>> + ret = delete_boot_option(bootorder, i, bootorder_size); > >>> + if (ret != EFI_SUCCESS) { > >>> + free(load_option); > >>> + goto out; > >>> + } > >>> + > >>> + num--; > >>> + bootorder_size -= sizeof(u16); > >>> + free(load_option); > >>> + continue; > >>> + } > >>> + } > >>> +next: > >>> + free(load_option); > >>> + i++; > >>> + } > >>> + > >>> +out: > >>> + return ret; > >>> +} > >> >
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 860cb83182..970db3ee01 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -396,6 +396,89 @@ static int is_blk_device_available(char *token) return -ENODEV; } +/** + * prepare_media_device_entry() - generate the media device entries + * + * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL + * and generate the bootmenu entries. + * This function also provide the BOOT#### variable maintenance for + * the media device entries. + * - Automatically create the BOOT#### variable for the newly detected device, + * this BOOT#### variable is distinguished by the special GUID + * stored in the EFI_LOAD_OPTION.optional_data + * - If the device is not attached to the system, the associated BOOT#### variable + * is automatically deleted. + * + * Return: status code + */ +static efi_status_t prepare_media_device_entry(void) +{ + u32 i; + efi_status_t ret; + efi_uintn_t count; + efi_handle_t *volume_handles = NULL; + struct efi_bootmenu_media_boot_option *opt = NULL; + + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, + NULL, &count, (efi_handle_t **)&volume_handles); + if (ret != EFI_SUCCESS) + return ret; + + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); + if (!opt) + goto out; + + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); + if (ret != EFI_SUCCESS) + goto out; + + /* + * System hardware configuration may vary depending on the user setup. + * The boot option is automatically added by the bootmenu. + * If the device is not attached to the system, the boot option needs + * to be deleted. + */ + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); + if (ret != EFI_SUCCESS) + goto out; + + /* add non-existent boot option */ + for (i = 0; i < count; i++) { + u32 boot_index; + u16 var_name[9]; + + if (!opt[i].exist) { + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), + &boot_index); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + opt[i].size, opt[i].lo, false); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_bootmenu_append_bootorder(boot_index); + if (ret != EFI_SUCCESS) + goto out; + } + } + +out: + if (opt) { + for (i = 0; i < count; i++) + free(opt[i].lo); + } + free(opt); + efi_free_pool(volume_handles); + + return ret; +} + /** * prepare_distro_boot_entry() - generate the distro boot entries * @@ -500,6 +583,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, static struct bootmenu_data *bootmenu_create(int delay) { int ret; + efi_status_t efi_ret; unsigned short int i = 0; struct bootmenu_data *menu; struct bootmenu_entry *iter = NULL; @@ -523,6 +607,16 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup; if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { + if (i < MAX_DYNAMIC_ENTRY) { + /* + * UEFI specification requires booting from removal media using + * a architecture-specific default image name such as BOOTAA64.EFI. + */ + efi_ret = prepare_media_device_entry(); + if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND) + goto cleanup; + } + if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT) diff --git a/include/efi_loader.h b/include/efi_loader.h index 533618341b..bef492ccb9 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -928,6 +928,22 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message; +/** + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device + * + * This structure is used to enumerate possible boot option + * + * @lo: Serialized load option data + * @size: Size of serialized load option data + * @exist: Flag to indicate the load option already exists + * in Non-volatile load option + */ +struct efi_bootmenu_media_boot_option { + void *lo; + efi_uintn_t size; + bool exist; +}; + bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db, bool dbx); @@ -1076,6 +1092,10 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size, u32 *index); efi_status_t efi_bootmenu_append_bootorder(u16 index); +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_media_boot_option *opt, + efi_handle_t *volume_handles, efi_status_t count); +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, + efi_status_t count); efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c index 8c3f94c695..33b37fd11a 100644 --- a/lib/efi_loader/efi_bootmenu_maintenance.c +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -26,6 +26,13 @@ static struct efi_simple_text_output_protocol *cout; #define EFI_BOOTMENU_BOOT_NAME_MAX 32 #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 +#define EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID \ + EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \ + 0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde) + +static const efi_guid_t efi_guid_bootmenu_auto_generated = + EFI_BOOTMENU_AUTO_GENERATED_ENTRY_GUID; + typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); /** @@ -1104,3 +1111,135 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) ARRAY_SIZE(maintenance_menu_items), -1); } + +efi_status_t efi_bootmenu_enumerate_boot_option(struct efi_bootmenu_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++) { + char *optional_data; + u16 *dev_name, *p; + struct efi_load_option lo; + struct efi_block_io *block_io; + 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_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&block_io, + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); + if (!dev_name) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + 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 "dummystr" string to allocate enough space + * to store guid, instead of realloc the load_option. + * + * This will allocate 16 bytes for guid plus trailing 0x0000. + * The guid does not require trailing 0x0000, but it is for safety + * in case some program handle the optional_data as u16 string. + */ + lo.optional_data = "dummystr"; + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); + if (!opt[i].size) { + ret = EFI_OUT_OF_RESOURCES; + free(dev_name); + goto out; + } + /* set the guid */ + optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"dummystr")); + memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); + free(dev_name); + } + +out: + return ret; +} + +efi_status_t efi_bootmenu_delete_invalid_boot_option(struct efi_bootmenu_media_boot_option *opt, + efi_status_t count) +{ + u16 *bootorder; + u32 i, j; + efi_status_t ret; + efi_uintn_t num, size, bootorder_size; + void *load_option; + struct efi_load_option lo; + u16 varname[] = u"Boot####"; + + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); + if (!bootorder) + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ + + num = bootorder_size / sizeof(u16); + for (i = 0; i < num;) { + efi_uintn_t tmp; + + efi_create_indexed_name(varname, sizeof(varname), + "Boot", bootorder[i]); + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); + if (!load_option) + goto next; + + tmp = size; + ret = efi_deserialize_load_option(&lo, load_option, &tmp); + if (ret != EFI_SUCCESS) + goto next; + + if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { + for (j = 0; j < count; j++) { + if (memcmp(opt[j].lo, load_option, size) == 0) { + opt[j].exist = true; + break; + } + } + + if (j == count) { + ret = delete_boot_option(bootorder, i, bootorder_size); + if (ret != EFI_SUCCESS) { + free(load_option); + goto out; + } + + num--; + bootorder_size -= sizeof(u16); + free(load_option); + continue; + } + } +next: + free(load_option); + i++; + } + +out: + return ret; +}
UEFI specification requires booting from removal media using a architecture-specific default image name such as BOOTAA64.EFI. This commit adds the removable media entries into bootmenu, so that user can select the removable media and boot with default image. The bootmenu automatically enumerates the possible bootable media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, add it as new UEFI boot option(BOOT####) and update BootOrder variable. This automatically generated UEFI boot option has the dedicated guid in the optional_data to distinguish it from the UEFI boot option user adds manually. This commit also provides the BOOT#### variable maintenance feature. Depending on the system hardware setup, some devices may not exist at a later system boot, so bootmenu checks the available device in each bootmenu invocation and automatically removes the BOOT#### variable corrensponding to the non-existent media device. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- Changes in v5: - Return EFI_SUCCESS if there is no BootOrder defined - correctly handle the case if no removable device found - use guid to identify the automatically generated entry by bootmenu Newly created in v4 cmd/bootmenu.c | 94 +++++++++++++++ include/efi_loader.h | 20 ++++ lib/efi_loader/efi_bootmenu_maintenance.c | 139 ++++++++++++++++++++++ 3 files changed, 253 insertions(+)