The message emitted by pre_parse() pretty clearly states the intention.
Make sure we actually do so.

Fixes: bf6501a62e80 ("x86-64: EFI boot code")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -907,8 +907,13 @@ static void __init pre_parse(const struc
             start = 0;
     }
     if ( file->size && end[-1] )
+    {
          PrintStr(L"No newline at end of config file,"
                    " last line will be ignored.\r\n");
+
+         for ( UINTN pos = file->size; pos-- && *--end; )
+             *end = 0;
+    }
 }
 
 static void __init init_secure_boot_mode(void)

On 24/03/2026 4:36 pm, Jan Beulich wrote:
> The message emitted by pre_parse() pretty clearly states the intention.
> Make sure we actually do so.
>
> Fixes: bf6501a62e80 ("x86-64: EFI boot code")
> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -907,8 +907,13 @@ static void __init pre_parse(const struc
>              start = 0;
>      }
>      if ( file->size && end[-1] )
> +    {
>           PrintStr(L"No newline at end of config file,"
>                     " last line will be ignored.\r\n");
> +
> +         for ( UINTN pos = file->size; pos-- && *--end; )
> +             *end = 0;
> +    }

I agree this is what the the function intended.

But, ignoring the final line is rude and there's no viable editor in a
UEFI shell to fix it.  Can't we just copy the file into a
one-byte-bigger buffer and terminate it properly?

~Andrew

On 24.03.2026 18:13, Andrew Cooper wrote:
> On 24/03/2026 4:36 pm, Jan Beulich wrote:
>> The message emitted by pre_parse() pretty clearly states the intention.
>> Make sure we actually do so.
>>
>> Fixes: bf6501a62e80 ("x86-64: EFI boot code")
>> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -907,8 +907,13 @@ static void __init pre_parse(const struc
>>              start = 0;
>>      }
>>      if ( file->size && end[-1] )
>> +    {
>>           PrintStr(L"No newline at end of config file,"
>>                     " last line will be ignored.\r\n");
>> +
>> +         for ( UINTN pos = file->size; pos-- && *--end; )
>> +             *end = 0;
>> +    }
> 
> I agree this is what the the function intended.
> 
> But, ignoring the final line is rude and there's no viable editor in a
> UEFI shell to fix it.

On all the early EFI systems I had in use there was one. I'd need to check
more recent systems, though. (I know some come without any EFI shell at all.)

>  Can't we just copy the file into a
> one-byte-bigger buffer and terminate it properly?

If that would have been possible with equally little churn, I would have
done it that way. Of course it is possible in principle.

Jan

On 25.03.2026 07:12, Jan Beulich wrote:
> On 24.03.2026 18:13, Andrew Cooper wrote:
>> On 24/03/2026 4:36 pm, Jan Beulich wrote:
>>> The message emitted by pre_parse() pretty clearly states the intention.
>>> Make sure we actually do so.
>>>
>>> Fixes: bf6501a62e80 ("x86-64: EFI boot code")
>>> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/common/efi/boot.c
>>> +++ b/xen/common/efi/boot.c
>>> @@ -907,8 +907,13 @@ static void __init pre_parse(const struc
>>>              start = 0;
>>>      }
>>>      if ( file->size && end[-1] )
>>> +    {
>>>           PrintStr(L"No newline at end of config file,"
>>>                     " last line will be ignored.\r\n");
>>> +
>>> +         for ( UINTN pos = file->size; pos-- && *--end; )
>>> +             *end = 0;
>>> +    }
>>
>> I agree this is what the the function intended.
>>
>> But, ignoring the final line is rude and there's no viable editor in a
>> UEFI shell to fix it.
> 
> On all the early EFI systems I had in use there was one. I'd need to check
> more recent systems, though. (I know some come without any EFI shell at all.)
> 
>>   Can't we just copy the file into a
>> one-byte-bigger buffer and terminate it properly?
> 
> If that would have been possible with equally little churn, I would have
> done it that way. Of course it is possible in principle.

Actually, it doesn't look all that bad - let me give that a try.

Jan