Re: [PATCH] xen/efi: Fix crash with initial empty EFI options

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.07.2025 08:03, Frediano Ziglio wrote:
> On Mon, Jul 7, 2025 at 5:04 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 07.07.2025 17:51, Frediano Ziglio wrote:
>>> On Mon, Jul 7, 2025 at 4:42 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 07.07.2025 17:11, Frediano Ziglio wrote:
>>>>> --- a/xen/common/efi/boot.c
>>>>> +++ b/xen/common/efi/boot.c
>>>>> @@ -345,6 +345,7 @@ static unsigned int __init get_argv(unsigned int 
>>>>> argc, CHAR16 **argv,
>>>>>                                      VOID *data, UINTN size, UINTN 
>>>>> *offset,
>>>>>                                      CHAR16 **options)
>>>>>  {
>>>>> +    CHAR16 **const orig_argv = argv;
>>>>>      CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL, *cmdline = 
>>>>> NULL;
>>>>>      bool prev_sep = true;
>>>>>
>>>>> @@ -384,7 +385,7 @@ static unsigned int __init get_argv(unsigned int 
>>>>> argc, CHAR16 **argv,
>>>>>                  {
>>>>>                      cmdline = data + *offset;
>>>>>                      /* Cater for the image name as first component. */
>>>>> -                    ++argc;
>>>>> +                    ++argv;
>>>>
>>>> We're on the argc == 0 and argv == NULL path here. Incrementing NULL is UB,
>>>> if I'm not mistaken.
>>>
>>> Not as far as I know. Why?
>>
>> Increment and decrement operators are like additions. For additions the 
>> standard
>> says: "For addition, either both operands shall have arithmetic type, or one
>> operand shall be a pointer to an object type and the other shall have integer
>> type." Neither of the alternatives is true for NULL.
> 
> Yes and no. The expression here is not NULL + 1, but (CHAR16**)NULL +
> 1, hence the pointer has a type and so the expression is valid.

Sorry, meant to reply to this as well: That's not my understanding of the word
"object".

Jan