Re: [PATCH 2/2] efi_loader: use short-form DP for load options

On 3/2/22 10:44, AKASHI Takahiro wrote:

On Sat, Feb 26, 2022 at 12:56:51PM +0100, Heinrich Schuchardt wrote:

...

The GUID of partitions is sufficient for identification and will stay constant in the lifetime of a boot option. The preceding path of the device-path may change due to changes in the enumeration of devices. Therefore it is preferable to use the short-form of device-paths in load options. Adjust the 'efidebug boot add' command accordingly.

Since a "long" device path is still valid, I think that a user should be allowed to use a long device path especially if he or she wants to limit an interface for loading any image. Please add an option to efidebug to select a short or long path.

Furthermore, I would like to ask you to add a test, as you always require me to do so, for loading from a short path. Otherwise, the feature will never be exercised in CI loop.

-Takahiro Akashi

Dear Takahiro,

thanks for reviewing. In the next round I will alternative options -b/-B, -i/-I to choose between long and short device path.

I will create a Python test exposes a initrd via LoadFile2 protocol and then use initrddump.efi as binary payload and to check the content of the initrd.

Best regards

Heinrich

...

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

cmd/efidebug.c | 15 +++++++++++---- include/efi_loader.h | 3 ++- lib/efi_loader/efi_device_path.c | 21 +++++++++++++-------- 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 401d13cc4c..f62a4345fd 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -747,7 +747,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, const char *file)

{

• struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;

• struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL, *short_fp; struct efi_device_path *initrd_dp = NULL; efi_status_t ret; const struct efi_initrd_dp id_dp = {

@@ -771,9 +771,12 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, printf("Cannot create device path for "%s %s"\n", part, file); goto out; }

• short_fp = efi_dp_shorten(tmp_fp);

• if (!short_fp)

• short_fp = tmp_fp;
  
  

  initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,

• 		  tmp_fp);
  

• 		  short_fp);
  
  

  out: efi_free_pool(tmp_dp);

@@ -807,6 +810,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, size_t label_len, label_len16; u16 *label; struct efi_device_path *device_path = NULL, *file_path = NULL;

• struct efi_device_path *fp_free = NULL; struct efi_device_path *final_fp = NULL; struct efi_device_path *initrd_dp = NULL; struct efi_load_option lo;

@@ -849,13 +853,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,

	/* file path */
	ret = efi_dp_from_name(argv[3], argv[4], argv[5],

• 			       &device_path, &file_path);
  

• 			       &device_path, &fp_free);
  if (ret != EFI_SUCCESS) {
  	printf("Cannot create device path for \"%s %s\"\n",
  	       argv[3], argv[4]);
  	r = CMD_RET_FAILURE;
  	goto out;
  }
  

• 	file_path = efi_dp_shorten(fp_free);
  

• 	if (!file_path)
  

• 		file_path = fp_free;
  fp_size += efi_dp_size(file_path) +
  	sizeof(struct efi_device_path);
  argc -= 5;
  

@@ -927,7 +934,7 @@ out: efi_free_pool(final_fp); efi_free_pool(initrd_dp); efi_free_pool(device_path);

• efi_free_pool(file_path);

• efi_free_pool(fp_free); free(lo.label);

  return r;

diff --git a/include/efi_loader.h b/include/efi_loader.h index e390d323a9..c7d6b7c7f3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -725,7 +725,8 @@ extern void *efi_bounce_buffer; #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024) #endif

+/* shorten device path */ +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp); struct efi_device_path *efi_dp_next(const struct efi_device_path *dp); int efi_dp_match(const struct efi_device_path *a, const struct efi_device_path *b); diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index dc787b4d3d..ddd5f132ec 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -122,20 +122,25 @@ int efi_dp_match(const struct efi_device_path *a, } }

-/* +/**

• • efi_dp_shorten() - shorten device-path
• • We can have device paths that start with a USB WWID or a USB Class node,
  • and a few other cases which don't encode the full device path with bus
  • hierarchy:

• • • MESSAGING:USB_WWID
• • • MESSAGING:USB_CLASS
• • • MEDIA:FILE_PATH
• • • MEDIA:HARD_DRIVE
• • • MESSAGING:URI

• • • MESSAGING:USB_WWID
• • • MESSAGING:USB_CLASS
• • • MEDIA:FILE_PATH
• • • MEDIA:HARD_DRIVE
• • • MESSAGING:URI
  • See UEFI spec (section 3.1.2, about short-form device-paths)
• • @dp: original devie-path
• • @Return: shortened device-path or NULL
  */

-static struct efi_device_path *shorten_path(struct efi_device_path *dp) +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp) { while (dp) { /* @@ -189,7 +194,7 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, } }

• 	obj_dp = shorten_path(efi_dp_next(obj_dp));
  

• 	obj_dp = efi_dp_shorten(efi_dp_next(obj_dp));
  

  } while (short_path && obj_dp); }

-- 2.34.1