* [PATCH v3] disk: efi_loader: Improve disk image detection in efi_bootmgr
@ 2025-08-29 15:46 Javier Tia
2025-09-01 10:09 ` Ilias Apalodimas
0 siblings, 1 reply; 2+ messages in thread
From: Javier Tia @ 2025-08-29 15:46 UTC (permalink / raw)
To: u-boot, Tom Rini, Heinrich Schuchardt, Ilias Apalodimas
Enhances the process for identifying disk images within the EFI boot
manager. Utilize part_driver_lookup_type() to verify the validity of a
downloaded file as a disk image, rather than depending on file
extensions.
A temporary block device is created from the downloaded image and
part_driver_lookup_type() is used for comprehensive partition detection.
If a partition driver is found, the temporary device is reused instead
of being destroyed and recreated.
Signed-off-by: Javier Tia <javier.tia@linaro.org>
---
Changelog:
- v3 changes:
Update prepare_loaded_image() to require a non-null blk_dev parameter,
eliminating the conditional branching logic and replacing it with
direct assignment. In try_load_from_uri_path(), variable declarations
were moved to follow U-Boot Coding convention, redundant cleanup calls
were consolidated into a single location, and control flow was
improved by replacing empty conditional blocks with logical else
clauses for debug logging.
- v2 link: https://lore.kernel.org/u-boot/20250808213627.649669-1-javier.tia@linaro.org/
- v1 link: https://lore.kernel.org/u-boot/20250805162158.117876-1-javier.tia@linaro.org/
---
disk/part.c | 2 +-
include/part.h | 18 +++++++++
lib/efi_loader/efi_bootmgr.c | 78 +++++++++++++++++++++++++++---------
3 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 66e2b3a7219..f0c6866e210 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);
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..60e8804b1da 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>
@@ -346,19 +347,26 @@ static efi_status_t fill_default_file_path(struct udevice *blk,
* @size: image size
* @dp: pointer to default file device path
* @blk: pointer to created blk udevice
+ * @blk_dev: existing block device to use (must be non-NULL)
* Return: status code
+ *
+ * Caller must pass a valid non-NULL blk_dev; cleanup of temporary devices
+ * is caller's responsibility.
*/
static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
struct efi_device_path **dp,
- struct udevice **blk)
+ struct udevice **blk,
+ struct udevice *blk_dev)
{
u64 pages;
efi_status_t ret;
struct udevice *ramdisk_blk;
- ramdisk_blk = mount_image(label, addr, size);
- if (!ramdisk_blk)
- return EFI_LOAD_ERROR;
+ if (!blk_dev)
+ return EFI_INVALID_PARAMETER;
+
+ /* Use the provided block device */
+ ramdisk_blk = blk_dev;
ret = fill_default_file_path(ramdisk_blk, dp);
if (ret != EFI_SUCCESS) {
@@ -387,9 +395,7 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
return EFI_SUCCESS;
err:
- if (blkmap_destroy(ramdisk_blk->parent))
- log_err("Destroying blkmap failed\n");
-
+ /* Cleanup is now the caller's responsibility */
return ret;
}
@@ -407,7 +413,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 +458,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 +486,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;
@@ -476,6 +495,8 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
efi_handle_t mem_handle = NULL;
struct efi_device_path *loaded_dp;
static ulong image_size, image_addr;
+ struct udevice *temp_bdev;
+ struct part_driver *part_drv = NULL;
ctx = calloc(1, sizeof(struct uridp_context));
if (!ctx)
@@ -516,17 +537,34 @@ 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)) {
+
+ /* Create temporary block device from the downloaded image for testing */
+ temp_bdev = mount_image(lo_label, image_addr, image_size);
+ if (temp_bdev) {
+ struct blk_desc *desc = dev_get_uclass_plat(temp_bdev);
+ if (!desc) {
+ cleanup_temp_blkdev(temp_bdev);
+ temp_bdev = NULL; // prevent double cleanup later
+ } else {
+ /* Use part_driver_lookup_type for comprehensive partition detection */
+ part_drv = part_driver_lookup_type(desc);
+ }
+ } else {
+ log_debug("mount_image() failed, will attempt PE-COFF detection\n");
+ }
+
+ if (part_drv) {
+ /* Reuse the temporary device instead of destroying and recreating */
ret = prepare_loaded_image(lo_label, image_addr, image_size,
- &loaded_dp, &blk);
- if (ret != EFI_SUCCESS)
+ &loaded_dp, &blk, temp_bdev);
+ if (ret != EFI_SUCCESS) {
+ cleanup_temp_blkdev(temp_bdev);
+ temp_bdev = NULL;
goto err;
+ }
source_buffer = NULL;
source_size = 0;
@@ -545,11 +583,15 @@ 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");
+ log_err("Error: disk image is not supported\n");
ret = EFI_UNSUPPORTED;
goto err;
}
+ /* Clean up temporary device if it was created but not used for disk image */
+ if (!part_drv)
+ cleanup_temp_blkdev(temp_bdev);
+
ctx->image_size = image_size;
ctx->image_addr = image_addr;
ctx->loaded_dp = loaded_dp;
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] disk: efi_loader: Improve disk image detection in efi_bootmgr
2025-08-29 15:46 [PATCH v3] disk: efi_loader: Improve disk image detection in efi_bootmgr Javier Tia
@ 2025-09-01 10:09 ` Ilias Apalodimas
0 siblings, 0 replies; 2+ messages in thread
From: Ilias Apalodimas @ 2025-09-01 10:09 UTC (permalink / raw)
To: Javier Tia; +Cc: u-boot, Tom Rini, Heinrich Schuchardt
Hi Javier,
On Fri, 29 Aug 2025 at 18:54, Javier Tia <javier.tia@linaro.org> wrote:
>
> Enhances the process for identifying disk images within the EFI boot
> manager. Utilize part_driver_lookup_type() to verify the validity of a
> downloaded file as a disk image, rather than depending on file
> extensions.
>
> A temporary block device is created from the downloaded image and
> part_driver_lookup_type() is used for comprehensive partition detection.
> If a partition driver is found, the temporary device is reused instead
> of being destroyed and recreated.
>
> Signed-off-by: Javier Tia <javier.tia@linaro.org>
>
> ---
> Changelog:
> - v3 changes:
> Update prepare_loaded_image() to require a non-null blk_dev parameter,
> eliminating the conditional branching logic and replacing it with
> direct assignment. In try_load_from_uri_path(), variable declarations
> were moved to follow U-Boot Coding convention, redundant cleanup calls
> were consolidated into a single location, and control flow was
> improved by replacing empty conditional blocks with logical else
> clauses for debug logging.
> - v2 link: https://lore.kernel.org/u-boot/20250808213627.649669-1-javier.tia@linaro.org/
> - v1 link: https://lore.kernel.org/u-boot/20250805162158.117876-1-javier.tia@linaro.org/
>
> ---
> disk/part.c | 2 +-
> include/part.h | 18 +++++++++
> lib/efi_loader/efi_bootmgr.c | 78 +++++++++++++++++++++++++++---------
> 3 files changed, 79 insertions(+), 19 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 66e2b3a7219..f0c6866e210 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);
> 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..60e8804b1da 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>
> @@ -346,19 +347,26 @@ static efi_status_t fill_default_file_path(struct udevice *blk,
> * @size: image size
> * @dp: pointer to default file device path
> * @blk: pointer to created blk udevice
> + * @blk_dev: existing block device to use (must be non-NULL)
> * Return: status code
> + *
> + * Caller must pass a valid non-NULL blk_dev; cleanup of temporary devices
> + * is caller's responsibility.
> */
> static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
> struct efi_device_path **dp,
> - struct udevice **blk)
> + struct udevice **blk,
> + struct udevice *blk_dev)
> {
> u64 pages;
> efi_status_t ret;
> struct udevice *ramdisk_blk;
>
> - ramdisk_blk = mount_image(label, addr, size);
So, why can't we keep the mount_imagte() here now?
> - if (!ramdisk_blk)
> - return EFI_LOAD_ERROR;
> + if (!blk_dev)
> + return EFI_INVALID_PARAMETER;
> +
> + /* Use the provided block device */
> + ramdisk_blk = blk_dev;
>
> ret = fill_default_file_path(ramdisk_blk, dp);
> if (ret != EFI_SUCCESS) {
> @@ -387,9 +395,7 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size,
> return EFI_SUCCESS;
>
> err:
> - if (blkmap_destroy(ramdisk_blk->parent))
> - log_err("Destroying blkmap failed\n");
> -
> + /* Cleanup is now the caller's responsibility */
This is an error I think we are fine cleaning up here
> return ret;
> }
>
> @@ -407,7 +413,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 +458,20 @@ static void EFIAPI efi_bootmgr_http_return(struct efi_event *event,
> EFI_EXIT(ret);
> }
>
[...]
> *
> @@ -466,7 +486,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;
> @@ -476,6 +495,8 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
> efi_handle_t mem_handle = NULL;
> struct efi_device_path *loaded_dp;
> static ulong image_size, image_addr;
> + struct udevice *temp_bdev;
> + struct part_driver *part_drv = NULL;
>
> ctx = calloc(1, sizeof(struct uridp_context));
> if (!ctx)
> @@ -516,17 +537,34 @@ 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)) {
> +
> + /* Create temporary block device from the downloaded image for testing */
> + temp_bdev = mount_image(lo_label, image_addr, image_size);
> + if (temp_bdev) {
> + struct blk_desc *desc = dev_get_uclass_plat(temp_bdev);
> + if (!desc) {
> + cleanup_temp_blkdev(temp_bdev);
> + temp_bdev = NULL; // prevent double cleanup later
> + } else {
> + /* Use part_driver_lookup_type for comprehensive partition detection */
> + part_drv = part_driver_lookup_type(desc);
Can't we fold this into the prepare_image call?
then the code below becomes
ret = prepare_loaded_image(....);
if (ret == EFI_SUCCESS) {
struct blk_desc *desc = dev_get_uclass_plat(temp_bdev);
part_drv = part_driver_lookup_type(desc);
}
etc etc
> +
> + if (part_drv) {
> + /* Reuse the temporary device instead of destroying and recreating */
> ret = prepare_loaded_image(lo_label, image_addr, image_size,
> - &loaded_dp, &blk);
> - if (ret != EFI_SUCCESS)
> + &loaded_dp, &blk, temp_bdev);
> + if (ret != EFI_SUCCESS) {
> + cleanup_temp_blkdev(temp_bdev);
> + temp_bdev = NULL;
> goto err;
> + }
>
> source_buffer = NULL;
> source_size = 0;
> @@ -545,11 +583,15 @@ 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");
> + log_err("Error: disk image is not supported\n");
> ret = EFI_UNSUPPORTED;
> goto err;
> }
>
> + /* Clean up temporary device if it was created but not used for disk image */
> + if (!part_drv)
> + cleanup_temp_blkdev(temp_bdev);
> +
> ctx->image_size = image_size;
> ctx->image_addr = image_addr;
> ctx->loaded_dp = loaded_dp;
> --
> 2.51.0
>
Thanks
/Ilias
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-09-01 10:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 15:46 [PATCH v3] disk: efi_loader: Improve disk image detection in efi_bootmgr Javier Tia
2025-09-01 10:09 ` Ilias Apalodimas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).