[U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue May 7 06:04:26 UTC 2019
On 5/7/19 7:16 AM, Heinrich Schuchardt wrote:
> On 5/7/19 3:53 AM, Takahiro Akashi wrote:
>> On Tue, Apr 30, 2019 at 08:11:15AM +0200, Heinrich Schuchardt wrote:
>>> The field boot OptionalData in structure _EFI_LOAD_OPTIONS is for binary
>>> data.
>>>
>>> When we use `efidebug boot add` we should convert the 5th argument from
>>> UTF-8 to UTF-16 before putting it into the BootXXXX variable.
>>
>> While optional_data holds u8 string in calling
>> efi_serialize_load_option(),
>> it holds u16 string in leaving from efi_deserialize_load_option().
>> We should handle it in a consistent way if you want to keep optional_data
>> as "const u8."
When communicating with Linux optional data may contain a u16 string.
But I cannot see were our coding is inconsistent.
Regards
Heinrich
>
> The patch is already merged so I will have to create a follow up patch.
>
> The UEFI spec has EFI_LOAD_OPTION.OptionalData as UINT8. So an odd
> number of bytes is a possibility.
>
> Best regards
>
> Heinrich
>
>>
>> Thanks,
>> -Takahiro Akashi
>>
>>> When printing boot variables with `efidebug boot dump` we should support
>>> the OptionalData being arbitrary binary data. So let's dump the data as
>>> hexadecimal values.
>>>
>>> Here is an example session protocol:
>>>
>>> => efidebug boot add 00a1 label1 scsi 0:1 doit1 'my option'
>>> => efidebug boot add 00a2 label2 scsi 0:1 doit2
>>> => efidebug boot dump
>>> Boot00A0:
>>> attributes: A-- (0x00000001)
>>> label: label1
>>> file_path: .../HD(1,MBR,0xeac4e18b,0x800,0x3fffe)/doit1
>>> data:
>>> 00000000: 6d 00 79 00 20 00 6f 00 70 00 74 00 69 00 6f 00 m.y.
>>> .o.p.t.i.o.
>>> 00000010: 6e 00 00 00 n...
>>> Boot00A1:
>>> attributes: A-- (0x00000001)
>>> label: label2
>>> file_path: .../HD(1,MBR,0xeac4e18b,0x800,0x3fffe)/doit2
>>> data:
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> v2:
>>> remove statement without effect in efi_serialize_load_option()
>>> ---
>>> cmd/efidebug.c | 27 +++++++++++++++++----------
>>> include/efi_loader.h | 2 +-
>>> lib/efi_loader/efi_bootmgr.c | 15 ++++++++-------
>>> 3 files changed, 26 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>>> index efe4ea873e..c4ac9dd634 100644
>>> --- a/cmd/efidebug.c
>>> +++ b/cmd/efidebug.c
>>> @@ -11,6 +11,7 @@
>>> #include <efi_loader.h>
>>> #include <environment.h>
>>> #include <exports.h>
>>> +#include <hexdump.h>
>>> #include <malloc.h>
>>> #include <search.h>
>>> #include <linux/ctype.h>
>>> @@ -545,7 +546,10 @@ static int do_efi_boot_add(cmd_tbl_t *cmdtp, int
>>> flag,
>>> + sizeof(struct efi_device_path); /* for END */
>>>
>>> /* optional data */
>>> - lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
>>> + if (argc < 6)
>>> + lo.optional_data = NULL;
>>> + else
>>> + lo.optional_data = (const u8 *)argv[6];
>>>
>>> size = efi_serialize_load_option(&lo, (u8 **)&data);
>>> if (!size) {
>>> @@ -615,12 +619,13 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int
>>> flag,
>>> /**
>>> * show_efi_boot_opt_data() - dump UEFI load option
>>> *
>>> - * @id: Load option number
>>> - * @data: Value of UEFI load option variable
>>> + * @id: load option number
>>> + * @data: value of UEFI load option variable
>>> + * @size: size of the boot option
>>> *
>>> * Decode the value of UEFI load option variable and print
>>> information.
>>> */
>>> -static void show_efi_boot_opt_data(int id, void *data)
>>> +static void show_efi_boot_opt_data(int id, void *data, size_t size)
>>> {
>>> struct efi_load_option lo;
>>> char *label, *p;
>>> @@ -638,7 +643,7 @@ static void show_efi_boot_opt_data(int id, void
>>> *data)
>>> utf16_utf8_strncpy(&p, lo.label, label_len16);
>>>
>>> printf("Boot%04X:\n", id);
>>> - printf("\tattributes: %c%c%c (0x%08x)\n",
>>> + printf(" attributes: %c%c%c (0x%08x)\n",
>>> /* ACTIVE */
>>> lo.attributes & LOAD_OPTION_ACTIVE ? 'A' : '-',
>>> /* FORCE RECONNECT */
>>> @@ -646,14 +651,16 @@ static void show_efi_boot_opt_data(int id, void
>>> *data)
>>> /* HIDDEN */
>>> lo.attributes & LOAD_OPTION_HIDDEN ? 'H' : '-',
>>> lo.attributes);
>>> - printf("\tlabel: %s\n", label);
>>> + printf(" label: %s\n", label);
>>>
>>> dp_str = efi_dp_str(lo.file_path);
>>> - printf("\tfile_path: %ls\n", dp_str);
>>> + printf(" file_path: %ls\n", dp_str);
>>> efi_free_pool(dp_str);
>>>
>>> - printf("\tdata: %s\n", lo.optional_data);
>>> -
>>> + printf(" data:\n");
>>> + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
>>> + lo.optional_data, size + (u8 *)data -
>>> + (u8 *)lo.optional_data, true);
>>> free(label);
>>> }
>>>
>>> @@ -686,7 +693,7 @@ static void show_efi_boot_opt(int id)
>>> data));
>>> }
>>> if (ret == EFI_SUCCESS)
>>> - show_efi_boot_opt_data(id, data);
>>> + show_efi_boot_opt_data(id, data, size);
>>> else if (ret == EFI_NOT_FOUND)
>>> printf("Boot%04X: not found\n", id);
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index 39ed8a6fa5..07b913d256 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -560,7 +560,7 @@ struct efi_load_option {
>>> u16 file_path_length;
>>> u16 *label;
>>> struct efi_device_path *file_path;
>>> - u8 *optional_data;
>>> + const u8 *optional_data;
>>> };
>>>
>>> void efi_deserialize_load_option(struct efi_load_option *lo, u8
>>> *data);
>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>> index 4ccba22875..7bf51874c1 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -53,19 +53,20 @@ void efi_deserialize_load_option(struct
>>> efi_load_option *lo, u8 *data)
>>> */
>>> unsigned long efi_serialize_load_option(struct efi_load_option *lo,
>>> u8 **data)
>>> {
>>> - unsigned long label_len, option_len;
>>> + unsigned long label_len;
>>> unsigned long size;
>>> u8 *p;
>>>
>>> label_len = (u16_strlen(lo->label) + 1) * sizeof(u16);
>>> - option_len = strlen((char *)lo->optional_data);
>>>
>>> /* total size */
>>> size = sizeof(lo->attributes);
>>> size += sizeof(lo->file_path_length);
>>> size += label_len;
>>> size += lo->file_path_length;
>>> - size += option_len + 1;
>>> + if (lo->optional_data)
>>> + size += (utf8_utf16_strlen((const char *)lo->optional_data)
>>> + + 1) * sizeof(u16);
>>> p = malloc(size);
>>> if (!p)
>>> return 0;
>>> @@ -84,10 +85,10 @@ unsigned long efi_serialize_load_option(struct
>>> efi_load_option *lo, u8 **data)
>>> memcpy(p, lo->file_path, lo->file_path_length);
>>> p += lo->file_path_length;
>>>
>>> - memcpy(p, lo->optional_data, option_len);
>>> - p += option_len;
>>> - *(char *)p = '\0';
>>> -
>>> + if (lo->optional_data) {
>>> + utf8_utf16_strcpy((u16 **)&p, (const char *)lo->optional_data);
>>> + p += sizeof(u16); /* size of trailing \0 */
>>> + }
>>> return size;
>>> }
>>>
>>> --
>>> 2.20.1
>>>
>>
>
>
More information about the U-Boot
mailing list