[PATCH 09/10] Fix efi_bind_block.

[Ilias Apalodimas]

Hi Matthew, Janis,

On Sat, 23 Nov 2024 at 21:57, Matthew Garrett <mjg59 at srcf.ucam.org> 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.

Yep and I wonder how the device_unbind() code managed to survice since
it free that stack allocated memory...
Probably never called.

> 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;
> +}

We already define efi_dp_size(), you can use that here.

> +
>  /**
>   * 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)

you need to free plat here now

>                 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);
>
> --
> 2.47.0
>

Thanks
/Ilias