[PATCH 1/1] efi_loader: correct shortening of device-paths

[Ilias Apalodimas]

On Sun, 26 Mar 2023 at 21:42, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 3/26/23 12:45, Mark Kettenis wrote:
> >> From: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> Date: Sun, 26 Mar 2023 12:25:54 +0200
> >>
> >> We use short device-paths in boot options so that a file on a block device
> >> can be found independent of the port into which the device is plugged.
> >>
> >> Usb() device-path nodes only contain port and interface information and
> >> therefore cannot identify a block device.
> >> UsbWwi() device-path nodes contain the serial number of USB devices.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> ---
> >>   include/efi_api.h                |  1 +
> >>   lib/efi_loader/efi_device_path.c | 21 ++++++---------------
> >>   2 files changed, 7 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/include/efi_api.h b/include/efi_api.h
> >> index c4512eeb86..dc6e5ce236 100644
> >> --- a/include/efi_api.h
> >> +++ b/include/efi_api.h
> >> @@ -604,6 +604,7 @@ struct efi_device_path_acpi_path {
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR        0x0b
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_UART            0x0e
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS       0x0f
> >> +#  define DEVICE_PATH_SUB_TYPE_MSG_USB_WWI  0x10
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_SATA            0x12
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_NVME            0x17
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_URI             0x18
> >> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> >> index 288baa1ca7..9ed5e6273d 100644
> >> --- a/lib/efi_loader/efi_device_path.c
> >> +++ b/lib/efi_loader/efi_device_path.c
> >> @@ -92,17 +92,13 @@ 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:
> >> + * When creating a short boot option we want to use a device-path that is
> >> + * independent of the location where the block device is plugged in.
> >>    *
> >> - * * MESSAGING:USB_WWID
> >> - * * MESSAGING:USB_CLASS
> >> - * * MEDIA:FILE_PATH
> >> - * * MEDIA:HARD_DRIVE
> >> - * * MESSAGING:URI
> >> + * UsbWwi() nodes contain a serial number, hard drive paths a partition
> >> + * UUID. Both should be unique.
> >
> > This sentence makes no sense.  Do they contain a serial number, a hard
> > drive path *and* a partition UUID?  But then all *three* should be
> > unique.
>
> * UsbWWi() nodes contain a unique serial number
> * HD() nodes contain a unique UUID.
>
> Shortened device paths for files will only be created if the device path
> is not a block device. Currently neither the eficonfig nor the efidebug
> command allow to specify such a device path.

Mark just wants the comment adjusted, which I think makes sense.
Maybe we could use the EFI spec as-is? IOW
" For USB WWID, the boot manager must use the device vendor ID, device
product id, and serial number, and must match any USB device in the
system that contains this information"

Regards
/Ilias
>
> Best regards
>
> Heinrich
>
> >>    *
> >> - * See UEFI spec (section 3.1.2, about short-form device-paths)
> >> + * See UEFI spec, section 3.1.2 for "short-form device path".
> >>    *
> >>    * @dp:            original device-path
> >>    * @Return:        shortened device-path or NULL
> >> @@ -110,12 +106,7 @@ int efi_dp_match(const struct efi_device_path *a,
> >>   struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp)
> >>   {
> >>      while (dp) {
> >> -            /*
> >> -             * TODO: Add MESSAGING:USB_WWID and MESSAGING:URI..
> >> -             * in practice fallback.efi just uses MEDIA:HARD_DRIVE
> >> -             * so not sure when we would see these other cases.
> >> -             */
> >> -            if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
> >> +            if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_WWI) ||
> >>                  EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
> >>                  EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
> >>                      return dp;
> >> --
> >> 2.39.2
> >>
> >>