Re: [U-Boot] [U-Boot,2/6] efi_loader: add efi_dp_from_name()

On 10/17/2018 09:32 AM, AKASHI Takahiro wrote:

Factor out efi_set_bootdev() and extract efi_dp_from_name(). This function will be used to set a boot device in efishell command.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

cmd/bootefi.c | 42 ++++++---------------------- include/efi_loader.h | 4 +++ lib/efi_loader/efi_device_path.c | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 34 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 99f5b2b95706..b3e2845120fe 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -647,45 +647,19 @@ U_BOOT_CMD(

void efi_set_bootdev(const char *dev, const char *devnr, const char *path) {

• char filename[32] = { 0 }; /* dp->str is u16[32] long */
• char *s;

• struct efi_device_path *device, *image;

• efi_status_t ret;

  /* efi_set_bootdev is typically called repeatedly, recover memory */ efi_free_pool(bootefi_device_path); efi_free_pool(bootefi_image_path);

• /* If blk_get_device_part_str fails, avoid duplicate free. */
• bootefi_device_path = NULL;
• bootefi_image_path = NULL;
• if (strcmp(dev, "Net")) {

• struct blk_desc *desc;
  

• disk_partition_t fs_partition;
  

• int part;
  

• part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
  

• 			       1);
  

• if (part < 0)
  

• 	return;
  

• bootefi_device_path = efi_dp_from_part(desc, part);
  

• } else {

-#ifdef CONFIG_NET

• bootefi_device_path = efi_dp_from_eth();
  

-#endif

• }

• if (!path)

• return;
  
  

• if (strcmp(dev, "Net")) {

• /* Add leading / to fs paths, because they're absolute */
  

• snprintf(filename, sizeof(filename), "/%s", path);
  

• ret = efi_dp_from_name(dev, devnr, path, &device, &image);
• if (ret == EFI_SUCCESS) {

• bootefi_device_path = device;
  

• bootefi_image_path = image;
  

  } else {

• snprintf(filename, sizeof(filename), "%s", path);
  

• bootefi_device_path = NULL;
  

• bootefi_image_path = NULL;
  

  }

• /* DOS style file path: */
• s = filename;
• while ((s = strchr(s, '/')))

• *s++ = '\\';
  

• bootefi_image_path = efi_dp_from_file(NULL, 0, filename);

} diff --git a/include/efi_loader.h b/include/efi_loader.h index 74df070316a0..b73fbb6de23f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -425,6 +425,10 @@ const struct efi_device_path *efi_dp_last_node( efi_status_t efi_dp_split_file_path(struct efi_device_path *full_path, struct efi_device_path **device_path, struct efi_device_path **file_path); +efi_status_t efi_dp_from_name(const char *dev, const char *devnr,

• 	      const char *path,
  

• 	      struct efi_device_path **device,
  

• 	      struct efi_device_path **file);
  
  

#define EFI_DP_TYPE(_dp, _type, _subtype) \ (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \ diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5a61a1c1dcf9..3af444147283 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -942,3 +942,50 @@ efi_status_t efi_dp_split_file_path(struct efi_device_path *full_path, *file_path = fp; return EFI_SUCCESS; }

+efi_status_t efi_dp_from_name(const char *dev, const char *devnr,

• 	      const char *path,
  

• 	      struct efi_device_path **device,
  

• 	      struct efi_device_path **file)
  

+{

• int is_net;
• struct blk_desc *desc = NULL;
• disk_partition_t fs_partition;
• int part = 0;
• char filename[32] = { 0 }; /* dp->str is u16[32] long */
• char *s;
• if (!device || (path && !file))

• return EFI_INVALID_PARAMETER;
  

• is_net = !strcmp(dev, "Net");
• if (!is_net) {

• part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
  

• 			       1);
  

• if (part < 0)
  

• 	return EFI_INVALID_PARAMETER;
  

• *device = efi_dp_from_part(desc, part);
  

• } else {

+#ifdef CONFIG_NET

• *device = efi_dp_from_eth();
  

+#endif

• }
• if (!path)

• return EFI_SUCCESS;
  

• if (!is_net) {

• /* Add leading / to fs paths, because they're absolute */
  

• snprintf(filename, sizeof(filename), "/%s", path);
  

This coding creates undefined behavior. You have to use two different variables.

• } else {

• snprintf(filename, sizeof(filename), "%s", path);
  

Same here.

@Alex: Please, remove the patch from efi-next until it is fixed.

Best regards

Heinrich

• }
• /* DOS style file path: */
• s = filename;
• while ((s = strchr(s, '/')))

• *s++ = '\\';
  

• *file = efi_dp_from_file(NULL, 0, filename);
• return EFI_SUCCESS;

+}