[PATCH] efi_loader: Change efi_dp_from_mem() to use size

[PATCH] efi_loader: Change efi_dp_from_mem() to use size

[Moritz Fischer]

All call sites are using size rather than end addresses,
so instead - as previously done - calculating an end address
everywhere, just modify the function to use size and internally
calculate the end address

Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Patrick Wildt <pwildt@google.com>
Signed-off-by: Moritz Fischer <moritzf@google.com>
---

Thinking about this some more it seems more sensible to fix this
in the function rather than patching all the call sites;
it seems we even missed another call site.

- Moritz

---
 include/efi_loader.h             | 2 +-
 lib/efi_loader/efi_bootbin.c     | 1 -
 lib/efi_loader/efi_device_path.c | 7 +++----
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 291eca5c07..39809eac1b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -858,7 +858,7 @@ struct efi_device_path *efi_dp_from_file(const struct efi_device_path *dp,
 struct efi_device_path *efi_dp_from_eth(void);
 struct efi_device_path *efi_dp_from_mem(uint32_t mem_type,
 					uint64_t start_address,
-					uint64_t end_address);
+					size_t size);
 /* Determine the last device path node that is not the end node. */
 const struct efi_device_path *efi_dp_last_node(
 			const struct efi_device_path *dp);
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
index bf38392fac..a87006b3c0 100644
--- a/lib/efi_loader/efi_bootbin.c
+++ b/lib/efi_loader/efi_bootbin.c
@@ -137,7 +137,6 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
 		 */
 		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
 					    (uintptr_t)source_buffer,
-					    (uintptr_t)source_buffer +
 					    source_size);
 		/*
 		 * Make sure that device for device_path exist
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index d7444588aa..ee387e1dfd 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -977,7 +977,7 @@ struct efi_device_path __maybe_unused *efi_dp_from_eth(void)
 /* Construct a device-path for memory-mapped image */
 struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
 					uint64_t start_address,
-					uint64_t end_address)
+					size_t size)
 {
 	struct efi_device_path_memory *mdp;
 	void *buf, *start;
@@ -992,7 +992,7 @@ struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
 	mdp->dp.length = sizeof(*mdp);
 	mdp->memory_type = memory_type;
 	mdp->start_address = start_address;
-	mdp->end_address = end_address;
+	mdp->end_address = start_address + size;
 	buf = &mdp[1];
 
 	*((struct efi_device_path *)buf) = END;
@@ -1073,8 +1073,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
 		efi_get_image_parameters(&image_addr, &image_size);
 
 		dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
-				     (uintptr_t)image_addr,
-				     (uintptr_t)image_addr + image_size);
+				     (uintptr_t)image_addr, image_size);
 	} else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
 		dp = efi_dp_from_eth();
 	} else if (!strcmp(dev, "Uart")) {
-- 
2.47.0.163.g1226f6d8fa-goog

Re: [PATCH] efi_loader: Change efi_dp_from_mem() to use size

[Heinrich Schuchardt]

On 11/4/24 02:49, Moritz Fischer wrote:
> All call sites are using size rather than end addresses,
> so instead - as previously done - calculating an end address
> everywhere, just modify the function to use size and internally
> calculate the end address
>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Patrick Wildt <pwildt@google.com>
> Signed-off-by: Moritz Fischer <moritzf@google.com>

This fixes an incorrect usage in lib/efi_loader/efi_bootmgr.c.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>
> Thinking about this some more it seems more sensible to fix this
> in the function rather than patching all the call sites;
> it seems we even missed another call site.
>
> - Moritz
>
> ---
>   include/efi_loader.h             | 2 +-
>   lib/efi_loader/efi_bootbin.c     | 1 -
>   lib/efi_loader/efi_device_path.c | 7 +++----
>   3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 291eca5c07..39809eac1b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -858,7 +858,7 @@ struct efi_device_path *efi_dp_from_file(const struct efi_device_path *dp,
>   struct efi_device_path *efi_dp_from_eth(void);
>   struct efi_device_path *efi_dp_from_mem(uint32_t mem_type,
>   					uint64_t start_address,
> -					uint64_t end_address);
> +					size_t size);
>   /* Determine the last device path node that is not the end node. */
>   const struct efi_device_path *efi_dp_last_node(
>   			const struct efi_device_path *dp);
> diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> index bf38392fac..a87006b3c0 100644
> --- a/lib/efi_loader/efi_bootbin.c
> +++ b/lib/efi_loader/efi_bootbin.c
> @@ -137,7 +137,6 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
>   		 */
>   		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
>   					    (uintptr_t)source_buffer,
> -					    (uintptr_t)source_buffer +
>   					    source_size);
>   		/*
>   		 * Make sure that device for device_path exist
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index d7444588aa..ee387e1dfd 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -977,7 +977,7 @@ struct efi_device_path __maybe_unused *efi_dp_from_eth(void)
>   /* Construct a device-path for memory-mapped image */
>   struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
>   					uint64_t start_address,
> -					uint64_t end_address)
> +					size_t size)
>   {
>   	struct efi_device_path_memory *mdp;
>   	void *buf, *start;
> @@ -992,7 +992,7 @@ struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
>   	mdp->dp.length = sizeof(*mdp);
>   	mdp->memory_type = memory_type;
>   	mdp->start_address = start_address;
> -	mdp->end_address = end_address;
> +	mdp->end_address = start_address + size;
>   	buf = &mdp[1];
>
>   	*((struct efi_device_path *)buf) = END;
> @@ -1073,8 +1073,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   		efi_get_image_parameters(&image_addr, &image_size);
>
>   		dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> -				     (uintptr_t)image_addr,
> -				     (uintptr_t)image_addr + image_size);
> +				     (uintptr_t)image_addr, image_size);
>   	} else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
>   		dp = efi_dp_from_eth();
>   	} else if (!strcmp(dev, "Uart")) {