[PATCH 1/1] efi_loader: efi_dp_part_node check dp_alloc return value

[Ilias Apalodimas]

On Thu, 6 Oct 2022 at 15:18, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 10/6/22 13:59, Ilias Apalodimas wrote:
> > On Thu, 6 Oct 2022 at 14:45, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> dp_alloc() may return NULL. This needs to be caught.
> >>
> >> Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node")
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> ---
> >>   lib/efi_loader/efi_device_path.c | 3 ++-
> >>   lib/efi_loader/efi_disk.c        | 5 +++++
> >>   2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> >> index ebffb77122..acae007f26 100644
> >> --- a/lib/efi_loader/efi_device_path.c
> >> +++ b/lib/efi_loader/efi_device_path.c
> >> @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part)
> >>                  dpsize = sizeof(struct efi_device_path_hard_drive_path);
> >>          buf = dp_alloc(dpsize);
> >>
> >> -       dp_part_node(buf, desc, part);
> >> +       if (buf)
> >> +               dp_part_node(buf, desc, part);
> >>
> >>          return buf;
> >>   }
> >> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> >> index 39ea1a68a6..79b28097b6 100644
> >> --- a/lib/efi_loader/efi_disk.c
> >> +++ b/lib/efi_loader/efi_disk.c
> >> @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev(
> >>                  struct efi_handler *handler;
> >>                  void *protocol_interface;
> >>
> >> +               if (!node) {
> >> +                       ret = EFI_OUT_OF_RESOURCES;
> >> +                       goto error;
> >> +               }
> >
> > There's a diskobj that has been allocated here and a handle we added.
> > We should clean those up too
>
> 512 error:
> 513         efi_delete_handle(&diskobj->header);
>
> What is missing here?

If deleting the handle fails we won't free that memory

Regards
/Ilias
>
> Best regards
>
> Heinrich