[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 07:12:56 UTC 2019
On 5/7/19 8:16 AM, Takahiro Akashi wrote:
> On Tue, May 07, 2019 at 08:04:26AM +0200, Heinrich Schuchardt wrote:
>> 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.
>
> I don't get your point.
> Do you want to allow 'u8 *' variable to hold u16 string?#
Yes, optional data may contain anything, in the case of Linux the
command line parameters as an u16 string.
Other operating systems may use the field in other ways, e.g. store an
ASCII string.
Regards
Heinrich
>
> -Takahiro Akashi
>
>> 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