On 05.08.25 18:21, Javier Tia wrote:
Refines the disk image detection mechanism in the EFI boot manager.
Instead of relying on file extensions, the updated code uses the
partition driver lookup function to determine if a downloaded file is a
valid disk image.

The change enhances the robustness of the boot manager by providing a
more accurate disk image detection. It also improves error handling by
adding a cleanup function for temporary block devices.

The partition driver lookup function is also made public to enable its
use in the EFI boot manager. The function documentation is updated
accordingly.

Signed-off-by: Javier Tia <javier....@linaro.org>
---
  disk/part.c                  |  3 +-
  include/part.h               | 18 +++++++++++
  lib/efi_loader/efi_bootmgr.c | 59 ++++++++++++++++++++++++++++++------
  3 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index 66e2b3a7219..adc24976624 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -63,7 +63,7 @@ static struct part_driver *part_driver_get_type(int part_type)
   * @dev_desc: Device descriptor
   * Return: Driver found, or NULL if none
   */
-static struct part_driver *part_driver_lookup_type(struct blk_desc *desc)
+struct part_driver *part_driver_lookup_type(struct blk_desc *desc)
  {
        struct part_driver *drv =
                ll_entry_start(struct part_driver, part_driver);
@@ -855,3 +855,4 @@ int part_get_bootable(struct blk_desc *desc)
return 0;
  }
+
diff --git a/include/part.h b/include/part.h
index b772fb34c8a..3fc3186f4c4 100644
--- a/include/part.h
+++ b/include/part.h
@@ -727,6 +727,24 @@ int part_get_type_by_name(const char *name);
   */
  int part_get_bootable(struct blk_desc *desc);
+/**
+ * part_driver_lookup_type() - Look up the partition driver for a blk device
+ *
+ * If @desc->part_type is PART_TYPE_UNKNOWN, this checks each partition driver
+ * against the blk device to see if there is a valid partition table acceptable
+ * to that driver.
+ *
+ * If @desc->part_type is already set, it just returns the driver for that
+ * type, without testing if the driver can find a valid partition on the
+ * descriptor.
+ *
+ * On success it updates @desc->part_type if set to PART_TYPE_UNKNOWN on entry
+ *
+ * @desc: Device descriptor
+ * Return: Driver found, or NULL if none
+ */
+struct part_driver *part_driver_lookup_type(struct blk_desc *desc);
+
  #else
  static inline int part_driver_get_count(void)
  { return 0; }
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 662993fb809..7acb4f4e92b 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -17,6 +17,7 @@
  #include <log.h>
  #include <malloc.h>
  #include <net.h>
+#include <part.h>
  #include <efi_loader.h>
  #include <efi_variable.h>
  #include <asm/unaligned.h>
@@ -407,7 +408,7 @@ static efi_status_t efi_bootmgr_release_uridp(struct 
uridp_context *ctx)
        if (!ctx)
                return ret;
- /* cleanup for iso or img image */
+       /* cleanup for disk image */
        if (ctx->ramdisk_blk_dev) {
                ret = efi_add_memory_map(ctx->image_addr, ctx->image_size,
                                         EFI_CONVENTIONAL_MEMORY);
@@ -452,6 +453,20 @@ static void EFIAPI efi_bootmgr_http_return(struct 
efi_event *event,
        EFI_EXIT(ret);
  }
+/**
+ * cleanup_temp_blkdev() - Clean up temporary block device
+ *
+ * @temp_bdev: temporary block device to clean up
+ */
+static void cleanup_temp_blkdev(struct udevice *temp_bdev)
+{
+       if (!temp_bdev)
+               return;
+
+       if (blkmap_destroy(temp_bdev->parent))
+               log_err("Destroying temporary blkmap failed\n");
+}
+
  /**
   * try_load_from_uri_path() - Handle the URI device path
   *
@@ -466,7 +481,6 @@ static efi_status_t try_load_from_uri_path(struct 
efi_device_path_uri *uridp,
  {
        char *s;
        int err;
-       int uri_len;
        efi_status_t ret;
        void *source_buffer;
        efi_uintn_t source_size;
@@ -516,13 +530,37 @@ static efi_status_t try_load_from_uri_path(struct 
efi_device_path_uri *uridp,
        image_size = ALIGN(image_size, SZ_2M);
/*
-        * If the file extension is ".iso" or ".img", mount it and try to load
-        * the default file.
-        * If the file is PE-COFF image, load the downloaded file.
+        * Check if the downloaded file is a disk image or PE-COFF image.
+        * Use partition driver lookup for disk image detection.
         */
-       uri_len = strlen(uridp->uri);
-       if (!strncmp(&uridp->uri[uri_len - 4], ".iso", 4) ||
-           !strncmp(&uridp->uri[uri_len - 4], ".img", 4)) {
+       struct udevice *temp_bdev;
+       struct part_driver *part_drv = NULL;
+
+       /* Create temporary block device from the downloaded image for testing 
*/
+       temp_bdev = mount_image(lo_label, image_addr, image_size);
+       if (!temp_bdev) {
+               log_debug("mount_image() failed, will attempt PE-COFF 
detection\n");
+       }
+       if (temp_bdev) {
+               struct blk_desc *desc = dev_get_uclass_plat(temp_bdev);
+               if (!desc) {
+                       /* Clean up temporary device when dev_get_uclass_plat() 
returns NULL */
+                       cleanup_temp_blkdev(temp_bdev);
+               } else {
+                       /* Use part_driver_lookup_type for comprehensive 
partition detection */
+                       part_drv = part_driver_lookup_type(desc);
+
+                       /*
+                        * If it's a disk image with valid partition type,
+                        * prepare_loaded_image will create the final block 
device
+                        */
+                       if (part_drv)
+                               /* Clean up temporary device, 
prepare_loaded_image will create a new one */
+                               cleanup_temp_blkdev(temp_bdev);

Why wouldn't you remove the temporary disk if there is no partition driver?

Why wouldn't we keep the temporary disk if it is usable instead of creating a new one?

Best regards

Heinrich

+               }
+       }
+
+       if (part_drv) {
                ret = prepare_loaded_image(lo_label, image_addr, image_size,
                                           &loaded_dp, &blk);
                if (ret != EFI_SUCCESS)
@@ -545,7 +583,10 @@ static efi_status_t try_load_from_uri_path(struct 
efi_device_path_uri *uridp,
                source_buffer = (void *)image_addr;
                source_size = image_size;
        } else {
-               log_err("Error: file type is not supported\n");
+               /* Clean up temporary device if it was created but not a valid 
disk image */
+               cleanup_temp_blkdev(temp_bdev);
+
+               log_err("Error: disk image is not supported\n");
                ret = EFI_UNSUPPORTED;
                goto err;
        }

Reply via email to