On Fri, 10 Nov 2023 at 15:50, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san
Going through the patch one last time before I merge it I noticed some memory leaks
On Fri, 10 Nov 2023 at 06:27, Masahisa Kojima masahisa.kojima@linaro.org wrote:
This supports to boot from the URI device path. When user selects the URI device path, bootmgr downloads the file using wget into the address specified by loadaddr env variable. If the file is .iso or .img file, mount the image with blkmap then try to boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI). Since boot option indicating the default file is automatically created when new disk is detected, system can boot by selecting the automatically created blkmap boot option. If the file is PE-COFF file, load and start the downloaded file.
The buffer used to download the ISO image file must be reserved to avoid the unintended access to the image and expose the ramdisk to the OS. For PE-COFF file case, this memory reservation is done in LoadImage Boot Service.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
[...]
+/**
- efi_bootmgr_image_return_notify() - return to efibootmgr callback
- @event: the event for which this notification function is registered
- @context: event context
- */
+static void EFIAPI efi_bootmgr_image_return_notify(struct efi_event *event,
void *context)+{
efi_status_t ret;EFI_ENTRY("%p, %p", event, context);ret = efi_bootmgr_release_uridp_resource(context);EFI_EXIT(ret);+}
+/**
- try_load_from_uri_path() - Handle the URI device path
- @uridp: uri device path
- @lo_label: label of load option
- @handle: pointer to handle for newly installed image
- Return: status code
- */
+static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
u16 *lo_label,efi_handle_t *handle)+{
char *s;int err;int uri_len;efi_status_t ret;void *source_buffer;efi_uintn_t source_size;struct uridp_context *ctx;struct udevice *blk = NULL;struct efi_event *event = NULL;efi_handle_t mem_handle = NULL;struct efi_device_path *loaded_dp;static ulong image_size, image_addr;ctx = calloc(1, sizeof(struct uridp_context));if (!ctx)return EFI_OUT_OF_RESOURCES;ctx is allocated here
s = env_get("loadaddr");if (!s) {log_err("Error: loadaddr is not set\n");return EFI_INVALID_PARAMETER;Should be freed here
}image_addr = hextoul(s, NULL);err = wget_with_dns(image_addr, uridp->uri);if (err < 0)return EFI_INVALID_PARAMETER;and here
image_size = env_get_hex("filesize", 0);if (!image_size)return EFI_INVALID_PARAMETER;and here
/** 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.*/uri_len = strlen(uridp->uri);if (!strncmp(&uridp->uri[uri_len - 4], ".iso", 4) ||!strncmp(&uridp->uri[uri_len - 4], ".img", 4)) {ret = prepare_loaded_image(lo_label, image_addr, image_size,&loaded_dp, &blk);if (ret != EFI_SUCCESS)goto err;source_buffer = NULL;source_size = 0;} else if (efi_check_pe((void *)image_addr, image_size, NULL) == EFI_SUCCESS) {/** loaded_dp must exist until efi application returns,* will be freed in return_to_efibootmgr event callback.*/loaded_dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,(uintptr_t)image_addr, image_size);ret = efi_install_multiple_protocol_interfaces(&mem_handle, &efi_guid_device_path, loaded_dp, NULL);if (ret != EFI_SUCCESS)goto err;source_buffer = (void *)image_addr;source_size = image_size;} else {log_err("Error: file type is not supported\n");return EFI_UNSUPPORTED;and here
[...]
Should we just goto err and set the return value instead of returning immediately?
Yes, we can set the return value goto err.
If yes I can fix that during the merge
Thank you. It is very helpful if you can fix this.
Thanks, Masahisa Kojima
Thanks /Ilias