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

[Heinrich Schuchardt]

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?

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>
>>