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 <[email protected]>
---
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





_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to