[U-Boot] [PATCH v2 1/1] efi_loader: optional data in load options are binary
Takahiro Akashi
takahiro.akashi at linaro.org
Tue May 7 06:16:11 UTC 2019
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 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