[PATCH 09/10] Fix efi_bind_block.

[Heinrich Schuchardt]

On 23.11.24 20:55, Matthew Garrett wrote:
> From: Janis Danisevskis <jdanisevskis at aurora.tech>
>
> efi_bind_block had two issues.
> 1. A pointer to a the stack was inserted as plat structure and thus used
> beyond its life time.
> 2. Only the first segment of the device path was copied into the
> platfom data structure resulting in an unterminated device path.
>
> Signed-off-by: Janis Danisevskis <jdanisevskis at aurora.tech>
> Signed-off-by: Matthew Garrett <mgarrett at aurora.tech>
> ---
>
>   lib/efi/efi_app_init.c | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c
> index 9704020b749..cc91e1d74b8 100644
> --- a/lib/efi/efi_app_init.c
> +++ b/lib/efi/efi_app_init.c
> @@ -19,6 +19,15 @@
>
>   DECLARE_GLOBAL_DATA_PTR;
>
> +static size_t device_path_length(const struct efi_device_path *device_path)
> +{
> +	const struct efi_device_path *d;
> +
> +	for (d = device_path; d->type != DEVICE_PATH_TYPE_END; d = (void *)d + d->length) {
> +	}
> +	return (void *)d - (void *)device_path + d->length;
> +}
> +
>   /**
>    * efi_bind_block() - bind a new block device to an EFI device
>    *
> @@ -39,19 +48,23 @@ int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
>   		   struct efi_device_path *device_path, int len,
>   		   struct udevice **devp)
>   {
> -	struct efi_media_plat plat;
> +	struct efi_media_plat *plat;
>   	struct udevice *dev;
>   	char name[18];
>   	int ret;
> -
> -	plat.handle = handle;
> -	plat.blkio = blkio;
> -	plat.device_path = malloc(device_path->length);
> -	if (!plat.device_path)
> +	size_t device_path_len = device_path_length(device_path);
> +
> +	plat = malloc(sizeof(struct efi_media_plat));
> +	if (!plat)
> +		return log_msg_ret("plat", -ENOMEM);
> +	plat->handle = handle;
> +	plat->blkio = blkio;
> +	plat->device_path = malloc(device_path_len);
> +	if (!plat->device_path)
>   		return log_msg_ret("path", -ENOMEM);
> -	memcpy(plat.device_path, device_path, device_path->length);
> +	memcpy(plat->device_path, device_path, device_path_len);
>   	ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
> -			  &plat, ofnode_null(), &dev);
> +			  plat, ofnode_null(), &dev);
>   	if (ret)
>   		return log_msg_ret("bind", ret);

Here you have a memory leak for pointer plat if ret != 0.

Simon suggested a follow up patch. Instead we should fix it in this patch.

@Simon
The logic in the caller setup_block() looks wrong.

LocateHandle() does not ensure that when you try to read from the block
device the same still does exist. E.g. if a USB device is removed the
block IO protocol could be uninstalled and the handle deleted and the
EFI app may crash. Please, call OpenProtocol() with attribute BY_DRIVER.

HandleProtocol() is considered deprecated. Please, use OpenProtocol()
instead.

Best regards

Heinrich