* [PATCH] efi: payload: image: fix many issues in the code.
@ 2025-09-04 1:23 chalianis1
2025-09-04 7:03 ` Ahmad Fatoum
0 siblings, 1 reply; 2+ messages in thread
From: chalianis1 @ 2025-09-04 1:23 UTC (permalink / raw)
To: s.hauer; +Cc: barebox, Chali Anis
From: Chali Anis <chalianis1@gmail.com>
fix memory free missing.
fix unrechable code issue.
set the fdt memory to a fixed 128KB.
fix oftree_file exist check.
Signed-off-by: Chali Anis <chalianis1@gmail.com>
---
efi/payload/image.c | 56 +++++++++++++++++++++++++++------------------
1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/efi/payload/image.c b/efi/payload/image.c
index 38d52a32ea64..9fcc27ffaa78 100644
--- a/efi/payload/image.c
+++ b/efi/payload/image.c
@@ -132,6 +132,7 @@ static int efi_load_file_image(const char *file,
size_t size;
efi_handle_t handle;
efi_status_t efiret = EFI_SUCCESS;
+ int ret;
buf = read_file(file, &size);
if (!buf)
@@ -141,7 +142,8 @@ static int efi_load_file_image(const char *file,
EFI_LOADER_CODE);
if (!exe) {
pr_err("Failed to allocate pages for image\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto free_buf;
}
memcpy(exe, buf, size);
@@ -149,25 +151,32 @@ static int efi_load_file_image(const char *file,
efiret = BS->load_image(false, efi_parent_image, efi_device_path, exe,
size, &handle);
if (EFI_ERROR(efiret)) {
+ ret = -efi_errno(efiret);
pr_err("failed to LoadImage: %s\n", efi_strerror(efiret));
- goto out;
- };
+ goto free_mem;
+ }
efiret = BS->open_protocol(handle, &efi_loaded_image_protocol_guid,
(void **)loaded_image, efi_parent_image,
NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
if (EFI_ERROR(efiret)) {
+ ret = -efi_errno(efiret);
pr_err("failed to OpenProtocol: %s\n", efi_strerror(efiret));
BS->unload_image(handle);
- goto out;
+ goto free_mem;
}
*h = handle;
+ free(buf);
return 0;
-out:
+
+free_mem:
efi_free_pages(exe, size);
- return -efi_errno(efiret);
+free_buf:
+ free(buf);
+
+ return ret;
}
static bool is_linux_image(enum filetype filetype, const void *base)
@@ -335,8 +344,8 @@ static bool ramdisk_is_fit(struct image_data *data)
return false;
}
- return data->os_fit ? (bool)fit_has_image(data->os_fit,
- data->fit_config, "ramdisk") : false;
+ return data->os_fit ? fit_has_image(data->os_fit,
+ data->fit_config, "ramdisk") > 0 : false;
}
static bool fdt_is_fit(struct image_data *data)
@@ -347,12 +356,12 @@ static bool fdt_is_fit(struct image_data *data)
return true;
if (data->oftree_file) {
- if (!stat(data->initrd_file, &st) && st.st_size > 0)
+ if (!stat(data->oftree_file, &st) && st.st_size > 0)
return false;
}
- return data->os_fit ? (bool)fit_has_image(data->os_fit,
- data->fit_config, "fdt") : false;
+ return data->os_fit ? fit_has_image(data->os_fit,
+ data->fit_config, "fdt") > 0 : false;
}
static int efi_load_os(struct efi_image_data *e)
@@ -364,12 +373,15 @@ static int efi_load_os(struct efi_image_data *e)
void *vmem = NULL;
int ret = 0;
- if (e->data->os_fit) {
- image = (void *)e->data->fit_kernel;
- image_size = e->data->fit_kernel_size;
- } else if (e->data->os_file)
+ if (!e->data->os_fit)
return efi_load_file_image(e->data->os_file,
- &e->loaded_image, &e->handle);
+ &e->loaded_image, &e->handle);
+
+ image = (void *)e->data->fit_kernel;
+ image_size = e->data->fit_kernel_size;
+
+ if (image_size <= 0 || !image)
+ return -EINVAL;
vmem = efi_allocate_pages(&mem, image_size, EFI_ALLOCATE_ANY_PAGES,
EFI_LOADER_CODE);
@@ -565,13 +577,13 @@ static int efi_load_fdt(struct efi_image_data *e)
of_tree = tmp;
}
- vmem = efi_allocate_pages(&mem, of_size + CONFIG_FDT_PADDING,
+ vmem = efi_allocate_pages(&mem, SZ_128K,
EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY);
if (!vmem) {
pr_err("Failed to allocate pages for FDT\n");
+ ret = -ENOMEM;
goto free_file;
- return -ENOMEM;
}
memcpy(vmem, of_tree, of_size);
@@ -585,17 +597,17 @@ static int efi_load_fdt(struct efi_image_data *e)
}
e->oftree_res.base = mem;
- e->oftree_res.size = of_size + CONFIG_FDT_PADDING;
+ e->oftree_res.size = SZ_128K;
- if (!from_fit && tmp)
+ if (!from_fit)
free(tmp);
return 0;
free_mem:
- efi_free_pages(vmem, of_size);
+ efi_free_pages(vmem, SZ_128K);
free_file:
- if (!from_fit && tmp)
+ if (!from_fit)
free(tmp);
return ret;
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] efi: payload: image: fix many issues in the code.
2025-09-04 1:23 [PATCH] efi: payload: image: fix many issues in the code chalianis1
@ 2025-09-04 7:03 ` Ahmad Fatoum
0 siblings, 0 replies; 2+ messages in thread
From: Ahmad Fatoum @ 2025-09-04 7:03 UTC (permalink / raw)
To: chalianis1, s.hauer; +Cc: barebox
Hi Anis,
Sidenote: majority of commit messages don't have a trailing period,
so you may want to drop it for uniformity.
On 04.09.25 03:23, chalianis1@gmail.com wrote:
> From: Chali Anis <chalianis1@gmail.com>
>
> fix memory free missing.
> fix unrechable code issue.
> set the fdt memory to a fixed 128KB.
> fix oftree_file exist check.
You appear to be patching code that's not yet upstream.
The version of efi/payload/image.c that's in next right now
has no oftree support.
Please squash the changes into your code and resend, but you can
wait a day until I had a chance to review it as well.
Cheers,
Ahmad
> Signed-off-by: Chali Anis <chalianis1@gmail.com>
> ---
> efi/payload/image.c | 56 +++++++++++++++++++++++++++------------------
> 1 file changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/efi/payload/image.c b/efi/payload/image.c
> index 38d52a32ea64..9fcc27ffaa78 100644
> --- a/efi/payload/image.c
> +++ b/efi/payload/image.c
> @@ -132,6 +132,7 @@ static int efi_load_file_image(const char *file,
> size_t size;
> efi_handle_t handle;
> efi_status_t efiret = EFI_SUCCESS;
> + int ret;
>
> buf = read_file(file, &size);
> if (!buf)
> @@ -141,7 +142,8 @@ static int efi_load_file_image(const char *file,
> EFI_LOADER_CODE);
> if (!exe) {
> pr_err("Failed to allocate pages for image\n");
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto free_buf;
> }
>
> memcpy(exe, buf, size);
> @@ -149,25 +151,32 @@ static int efi_load_file_image(const char *file,
> efiret = BS->load_image(false, efi_parent_image, efi_device_path, exe,
> size, &handle);
> if (EFI_ERROR(efiret)) {
> + ret = -efi_errno(efiret);
> pr_err("failed to LoadImage: %s\n", efi_strerror(efiret));
> - goto out;
> - };
> + goto free_mem;
> + }
>
> efiret = BS->open_protocol(handle, &efi_loaded_image_protocol_guid,
> (void **)loaded_image, efi_parent_image,
> NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> if (EFI_ERROR(efiret)) {
> + ret = -efi_errno(efiret);
> pr_err("failed to OpenProtocol: %s\n", efi_strerror(efiret));
> BS->unload_image(handle);
> - goto out;
> + goto free_mem;
> }
>
> *h = handle;
> + free(buf);
>
> return 0;
> -out:
> +
> +free_mem:
> efi_free_pages(exe, size);
> - return -efi_errno(efiret);
> +free_buf:
> + free(buf);
> +
> + return ret;
> }
>
> static bool is_linux_image(enum filetype filetype, const void *base)
> @@ -335,8 +344,8 @@ static bool ramdisk_is_fit(struct image_data *data)
> return false;
> }
>
> - return data->os_fit ? (bool)fit_has_image(data->os_fit,
> - data->fit_config, "ramdisk") : false;
> + return data->os_fit ? fit_has_image(data->os_fit,
> + data->fit_config, "ramdisk") > 0 : false;
> }
>
> static bool fdt_is_fit(struct image_data *data)
> @@ -347,12 +356,12 @@ static bool fdt_is_fit(struct image_data *data)
> return true;
>
> if (data->oftree_file) {
> - if (!stat(data->initrd_file, &st) && st.st_size > 0)
> + if (!stat(data->oftree_file, &st) && st.st_size > 0)
> return false;
> }
>
> - return data->os_fit ? (bool)fit_has_image(data->os_fit,
> - data->fit_config, "fdt") : false;
> + return data->os_fit ? fit_has_image(data->os_fit,
> + data->fit_config, "fdt") > 0 : false;
> }
>
> static int efi_load_os(struct efi_image_data *e)
> @@ -364,12 +373,15 @@ static int efi_load_os(struct efi_image_data *e)
> void *vmem = NULL;
> int ret = 0;
>
> - if (e->data->os_fit) {
> - image = (void *)e->data->fit_kernel;
> - image_size = e->data->fit_kernel_size;
> - } else if (e->data->os_file)
> + if (!e->data->os_fit)
> return efi_load_file_image(e->data->os_file,
> - &e->loaded_image, &e->handle);
> + &e->loaded_image, &e->handle);
> +
> + image = (void *)e->data->fit_kernel;
> + image_size = e->data->fit_kernel_size;
> +
> + if (image_size <= 0 || !image)
> + return -EINVAL;
>
> vmem = efi_allocate_pages(&mem, image_size, EFI_ALLOCATE_ANY_PAGES,
> EFI_LOADER_CODE);
> @@ -565,13 +577,13 @@ static int efi_load_fdt(struct efi_image_data *e)
> of_tree = tmp;
> }
>
> - vmem = efi_allocate_pages(&mem, of_size + CONFIG_FDT_PADDING,
> + vmem = efi_allocate_pages(&mem, SZ_128K,
> EFI_ALLOCATE_ANY_PAGES,
> EFI_ACPI_RECLAIM_MEMORY);
> if (!vmem) {
> pr_err("Failed to allocate pages for FDT\n");
> + ret = -ENOMEM;
> goto free_file;
> - return -ENOMEM;
> }
>
> memcpy(vmem, of_tree, of_size);
> @@ -585,17 +597,17 @@ static int efi_load_fdt(struct efi_image_data *e)
> }
>
> e->oftree_res.base = mem;
> - e->oftree_res.size = of_size + CONFIG_FDT_PADDING;
> + e->oftree_res.size = SZ_128K;
>
> - if (!from_fit && tmp)
> + if (!from_fit)
> free(tmp);
>
> return 0;
>
> free_mem:
> - efi_free_pages(vmem, of_size);
> + efi_free_pages(vmem, SZ_128K);
> free_file:
> - if (!from_fit && tmp)
> + if (!from_fit)
> free(tmp);
>
> return ret;
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-09-04 7:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-04 1:23 [PATCH] efi: payload: image: fix many issues in the code chalianis1
2025-09-04 7:03 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox