* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
@ 2019-04-30 6:11 Heinrich Schuchardt
2019-05-07 1:53 ` Takahiro Akashi
0 siblings, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-04-30 6:11 UTC (permalink / raw)
To: u-boot
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.
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@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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-04-30 6:11 [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary Heinrich Schuchardt
@ 2019-05-07 1:53 ` Takahiro Akashi
2019-05-07 5:16 ` Heinrich Schuchardt
0 siblings, 1 reply; 11+ messages in thread
From: Takahiro Akashi @ 2019-05-07 1:53 UTC (permalink / raw)
To: u-boot
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."
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@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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 1:53 ` Takahiro Akashi
@ 2019-05-07 5:16 ` Heinrich Schuchardt
2019-05-07 6:04 ` Heinrich Schuchardt
0 siblings, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-05-07 5:16 UTC (permalink / raw)
To: u-boot
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."
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@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
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 5:16 ` Heinrich Schuchardt
@ 2019-05-07 6:04 ` Heinrich Schuchardt
2019-05-07 6:16 ` Takahiro Akashi
0 siblings, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-05-07 6:04 UTC (permalink / raw)
To: u-boot
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@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
>>>
>>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 6:04 ` Heinrich Schuchardt
@ 2019-05-07 6:16 ` Takahiro Akashi
2019-05-07 7:12 ` Heinrich Schuchardt
0 siblings, 1 reply; 11+ messages in thread
From: Takahiro Akashi @ 2019-05-07 6:16 UTC (permalink / raw)
To: u-boot
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?
-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@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
> >>>
> >>
> >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 6:16 ` Takahiro Akashi
@ 2019-05-07 7:12 ` Heinrich Schuchardt
2019-05-07 7:30 ` Takahiro Akashi
0 siblings, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-05-07 7:12 UTC (permalink / raw)
To: u-boot
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@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
>>>>>
>>>>
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 7:12 ` Heinrich Schuchardt
@ 2019-05-07 7:30 ` Takahiro Akashi
2019-05-07 9:47 ` Graf, Alexander
2019-05-07 16:54 ` Heinrich Schuchardt
0 siblings, 2 replies; 11+ messages in thread
From: Takahiro Akashi @ 2019-05-07 7:30 UTC (permalink / raw)
To: u-boot
On Tue, May 07, 2019 at 09:12:56AM +0200, Heinrich Schuchardt wrote:
> 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.
The problem is that with your patch optional_data is *always* converted
to utf-16 as far as we use efidebug.
My efidebug is not for linux only.
-Takahiro Akashi
> 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@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
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 7:30 ` Takahiro Akashi
@ 2019-05-07 9:47 ` Graf, Alexander
2019-05-07 16:54 ` Heinrich Schuchardt
1 sibling, 0 replies; 11+ messages in thread
From: Graf, Alexander @ 2019-05-07 9:47 UTC (permalink / raw)
To: u-boot
On 07.05.19 09:30, Takahiro Akashi wrote:
> On Tue, May 07, 2019 at 09:12:56AM +0200, Heinrich Schuchardt wrote:
>> 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.
> The problem is that with your patch optional_data is *always* converted
> to utf-16 as far as we use efidebug.
> My efidebug is not for linux only.
So what does the UEFI Shell do for command argument passing? Does it
always pass in a utf16 string? If so, why?
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 7:30 ` Takahiro Akashi
2019-05-07 9:47 ` Graf, Alexander
@ 2019-05-07 16:54 ` Heinrich Schuchardt
2019-05-07 23:56 ` Takahiro Akashi
1 sibling, 1 reply; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-05-07 16:54 UTC (permalink / raw)
To: u-boot
On 5/7/19 9:30 AM, Takahiro Akashi wrote:
> On Tue, May 07, 2019 at 09:12:56AM +0200, Heinrich Schuchardt wrote:
>> 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.
>
> The problem is that with your patch optional_data is *always* converted
> to utf-16 as far as we use efidebug.
> My efidebug is not for linux only.
optional_data treated is not treated as u16 in efidebug:
include/hexdump.h:
void print_hex_dump(const char *prefix_str, int prefix_type,
int rowsize, int groupsize, const void *buf,
size_t len, bool ascii);
include/efi_loader:
struct efi_load_option {
u32 attributes;
u16 file_path_length;
u16 *label;
struct efi_device_path *file_path;
const u8 *optional_data;
};
cmd/efidebug.c
print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
lo.optional_data, size + (u8 *)data -
(u8 *)lo.optional_data, true);
Best regards
Heinrich
>
> -Takahiro Akashi
>
>
>> 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@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
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 16:54 ` Heinrich Schuchardt
@ 2019-05-07 23:56 ` Takahiro Akashi
2019-05-08 0:50 ` Heinrich Schuchardt
0 siblings, 1 reply; 11+ messages in thread
From: Takahiro Akashi @ 2019-05-07 23:56 UTC (permalink / raw)
To: u-boot
On Tue, May 07, 2019 at 06:54:45PM +0200, Heinrich Schuchardt wrote:
> On 5/7/19 9:30 AM, Takahiro Akashi wrote:
> >On Tue, May 07, 2019 at 09:12:56AM +0200, Heinrich Schuchardt wrote:
> >>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.
> >
> >The problem is that with your patch optional_data is *always* converted
> >to utf-16 as far as we use efidebug.
> >My efidebug is not for linux only.
>
> optional_data treated is not treated as u16 in efidebug:
With your patch,
efi_serialize_load_option() always convert a given argument to
utf-16 and then the resulting variable contains u16 string as
optional_data. On the other hand, efi_deserialize_load_option()
does *not* convert an encoded optional_data in a variable to utf-8.
See what I mean?
-Takahiro Akashi
> include/hexdump.h:
> void print_hex_dump(const char *prefix_str, int prefix_type,
> int rowsize, int groupsize, const void *buf,
> size_t len, bool ascii);
>
> include/efi_loader:
> struct efi_load_option {
> u32 attributes;
> u16 file_path_length;
> u16 *label;
> struct efi_device_path *file_path;
> const u8 *optional_data;
> };
>
> cmd/efidebug.c
> print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> lo.optional_data, size + (u8 *)data -
> (u8 *)lo.optional_data, true);
>
> Best regards
>
> Heinrich
>
> >
> >-Takahiro Akashi
> >
> >
> >>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@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
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
2019-05-07 23:56 ` Takahiro Akashi
@ 2019-05-08 0:50 ` Heinrich Schuchardt
0 siblings, 0 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2019-05-08 0:50 UTC (permalink / raw)
To: u-boot
On 5/8/19 1:56 AM, Takahiro Akashi wrote:
> On Tue, May 07, 2019 at 06:54:45PM +0200, Heinrich Schuchardt wrote:
>> On 5/7/19 9:30 AM, Takahiro Akashi wrote:
>>> On Tue, May 07, 2019 at 09:12:56AM +0200, Heinrich Schuchardt wrote:
>>>> 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.
>>>
>>> The problem is that with your patch optional_data is *always* converted
>>> to utf-16 as far as we use efidebug.
>>> My efidebug is not for linux only.
>>
>> optional_data treated is not treated as u16 in efidebug:
>
> With your patch,
> efi_serialize_load_option() always convert a given argument to
> utf-16 and then the resulting variable contains u16 string as
> optional_data. On the other hand, efi_deserialize_load_option()
> does *not* convert an encoded optional_data in a variable to utf-8.
>
When we use efi_serialize_load_option() we know that it is either an
UTF-8 string in the bootargs variable or UTF-8 command line parameter
for the `efi boot add` command. Currently we do not foresee adding
binary data. So we can simply convert the UTF-8 input to UTF-16 as will
be needed when passing the boot option to the operating system. (A patch
for the boot manager still to be delivered.)
But this is not the only way that BootXXXX variables can be set:
* An EFI application may be used to set an arbitrary binary string.
* In future an operating system could put some binary data into the load
option.
So it is not safe to convert the load option into UTF-8 for display.
That is why I chose to use print_hex_dump() for output. If your boot
option only contains ASCII letters it is still legible:
=> efi boot add f000 lable scsi 0:1 binary.efi 'my favorite option'
=> efi boot dump
BootF000:
attributes: A-- (0x00000001)
label: lable
file_path:
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x90381b6c,0x800,0x3fffe)/binary.efi
data:
00000000: 6d 00 79 00 20 00 66 00 61 00 76 00 6f 00 72 00 m.y.
.f.a.v.o.r.
00000010: 69 00 74 00 65 00 20 00 6f 00 70 00 74 00 69 00 i.t.e.
.o.p.t.i.
00000020: 6f 00 6e 00 00 00 o.n...
Best regards
Heinrich
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-08 0:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-30 6:11 [U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary Heinrich Schuchardt
2019-05-07 1:53 ` Takahiro Akashi
2019-05-07 5:16 ` Heinrich Schuchardt
2019-05-07 6:04 ` Heinrich Schuchardt
2019-05-07 6:16 ` Takahiro Akashi
2019-05-07 7:12 ` Heinrich Schuchardt
2019-05-07 7:30 ` Takahiro Akashi
2019-05-07 9:47 ` Graf, Alexander
2019-05-07 16:54 ` Heinrich Schuchardt
2019-05-07 23:56 ` Takahiro Akashi
2019-05-08 0:50 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox