[PATCH v2 1/8] efi_loader: allow concatenation with contained end node

[Ilias Apalodimas]

On Tue, 28 May 2024 at 19:59, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 28.05.24 18:18, Ilias Apalodimas wrote:
> > [...]
> >
> >>>> -               unsigned sz2 = efi_dp_size(dp2);
> >>>> +               size_t sz1;
> >>>> +               size_t sz2 = efi_dp_size(dp2);
> >>>>                   void *p;
> >>>>
> >>>> +               if (split_end_node < sizeof(struct efi_device_path))
> >>>
> >>> Can we be more explicit here pls? Someone might misuse this in the future
> >>> split_end_node < =1 ? And we can document we can use values up to
> >>> sizeof(struct efi_device_path) if we ever need extra functionality
> >>
> >> size_t split_end_node cannot be negative.
> >>
> >> The case split_end_node == 0 is handled below. What are you missing?
> >
> > someone misusing it and passing a value of '3' for example and print
> > an error if the value is
> > 1 < value < sizeof(struct efi_device_path)
>
> I would like to avoid over-engineering. How about
>
> -               if (split_end_node < sizeof(struct efi_device_path))
> +               if (split_end_node < 2)
>
> and changing the function description to refer to >= 2?

Nop, in that case, I prefer what's already there, simply because
values between 0 < sizeof(struct efi_device_path) will be used for
functionality similar to the one we have for 0,1 in the future

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> > Thanks
> > /Ilias
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>>> +                       sz1 = efi_dp_size(dp1);
> >>>> +               else
> >>>> +                       sz1 = split_end_node;
> >>>> +
> >>>>                   if (split_end_node)
> >>>>                           end_size = 2 * sizeof(END);
> >>>>                   else
> >>>> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
> >>>> index c95dbfa9b5f..ac250bbfcc9 100644
> >>>> --- a/lib/efi_loader/efi_device_path_utilities.c
> >>>> +++ b/lib/efi_loader/efi_device_path_utilities.c
> >>>> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path(
> >>>>           const struct efi_device_path *src2)
> >>>>    {
> >>>>           EFI_ENTRY("%pD, %pD", src1, src2);
> >>>> -       return EFI_EXIT(efi_dp_concat(src1, src2, false));
> >>>> +       return EFI_EXIT(efi_dp_concat(src1, src2, 0));
> >>>>    }
> >>>>
> >>>>    /*
> >>>> --
> >>>> 2.43.0
> >>>>
> >>>
> >>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> >>
>