[PATCH 1/1] efi_loader: error handling in efi_disk_add_dev

[Ilias Apalodimas]

On Sun, 30 Jul 2023 at 15:03, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> * If an error occurs in efi_disk_add_dev(), don't leak resources.
> * If calloc() fails while creating the file system protocol interface,
>   signal an error.
> * Rename efi_simple_file_system() to efi_create_simple_file_system().
> * Drop a little helpful debug message.
>
> Fixes: 2a92080d8c44 ("efi_loader: add file/filesys support")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  include/efi_loader.h      | 18 +++++++++++++++---
>  lib/efi_loader/efi_disk.c | 15 +++++++++------
>  lib/efi_loader/efi_file.c | 12 ++++++++----
>  3 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 3a64eb9c66..4a29ddaef4 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -696,9 +696,21 @@ void efi_signal_event(struct efi_event *event);
>  /* return true if the device is removable */
>  bool efi_disk_is_removable(efi_handle_t handle);
>
> -/* open file system: */
> -struct efi_simple_file_system_protocol *efi_simple_file_system(
> -               struct blk_desc *desc, int part, struct efi_device_path *dp);
> +/**
> + * efi_create_simple_file_system() - create simple file system protocol
> + *
> + * Create a simple file system protocol for a partition.
> + *
> + * @desc:      block device descriptor
> + * @part:      partition number
> + * @dp:                device path
> + * @fsp:       simple file system protocol
> + * Return:     status code
> + */
> +efi_status_t
> +efi_create_simple_file_system(struct blk_desc *desc, int part,
> +                             struct efi_device_path *dp,
> +                             struct efi_simple_file_system_protocol **fsp);
>
>  /* open file from device-path: */
>  struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 46cb5704a7..f0d76113b0 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -487,15 +487,16 @@ static efi_status_t efi_disk_add_dev(
>          */
>         if ((part || desc->part_type == PART_TYPE_UNKNOWN) &&
>             efi_fs_exists(desc, part)) {
> -               diskobj->volume = efi_simple_file_system(desc, part,
> -                                                        diskobj->dp);
> +               ret = efi_create_simple_file_system(desc, part, diskobj->dp,
> +                                                   &diskobj->volume);
> +               if (ret != EFI_SUCCESS)
> +                       goto error;
> +
>                 ret = efi_add_protocol(&diskobj->header,
>                                        &efi_simple_file_system_protocol_guid,
>                                        diskobj->volume);
> -               if (ret != EFI_SUCCESS) {
> -                       log_debug("simple FS failed\n");
> -                       return ret;
> -               }
> +               if (ret != EFI_SUCCESS)
> +                       goto error;
>         }
>         diskobj->ops = block_io_disk_template;
>         diskobj->dev_index = dev_index;
> @@ -538,6 +539,8 @@ static efi_status_t efi_disk_add_dev(
>         return EFI_SUCCESS;
>  error:
>         efi_delete_handle(&diskobj->header);
> +       free(diskobj->volume);
> +       free(diskobj);
>         return ret;
>  }
>
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index 520c730220..3764a92b50 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -1192,18 +1192,22 @@ efi_open_volume(struct efi_simple_file_system_protocol *this,
>         return EFI_EXIT(efi_open_volume_int(this, root));
>  }
>
> -struct efi_simple_file_system_protocol *
> -efi_simple_file_system(struct blk_desc *desc, int part,
> -                      struct efi_device_path *dp)
> +efi_status_t
> +efi_create_simple_file_system(struct blk_desc *desc, int part,
> +                             struct efi_device_path *dp,
> +                             struct efi_simple_file_system_protocol **fsp)
>  {
>         struct file_system *fs;
>
>         fs = calloc(1, sizeof(*fs));
> +       if (!fs)
> +               return EFI_OUT_OF_RESOURCES;
>         fs->base.rev = EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_REVISION;
>         fs->base.open_volume = efi_open_volume;
>         fs->desc = desc;
>         fs->part = part;
>         fs->dp = dp;
> +       *fsp = &fs->base;
>
> -       return &fs->base;
> +       return EFI_SUCCESS;
>  }
> --
> 2.40.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>