Re: grub-install deleting long UEFI entries bug ?

[adrian15]

El 23/04/17 a las 10:45, Andrei Borzenkov escribió:
> 23.04.2017 11:21, adrian15 adrian15 пишет:
>> 2017-04-23 6:36 GMT+02:00 Andrei Borzenkov <address@hidden>:
>>
>>> 23.04.2017 03:54, adrian15 пишет:
>>>> grub-install seems to be deleting long UEFI entries
>>>>
>>>> (*) What the bug is
>>>>
>>>> * Add an UEFI entry with this label (Remove the single quotes):
>>>>  '(Rescapp added) \EFI\ubuntu\MokManager.efi'
>>>>
>>>> Example:
>>>>
>>>> efibootmgr -c \
>>>>  -d /dev/sda \
>>>>  -p 2 \
>>>>  -L '(Rescapp added) \EFI\ubuntu\MokManager.efi' \
>>>>  -l '\EFI\ubuntu\MokManager.efi'
>>>>
>>>> * Run grub-install /dev/sda or maybe just grub-install
>>>>
>>>> I expect the newly added uefi entry to be there.
>>>> What I find is that the entry has been lost or deleted!
>>>>
>>>
>>> What is value of GRUB_DISTRIBUTOR in /etc/default/grub?
>>>
>>
>> After evaluating the bash expression the GRUB_DISTRIBUTOR value is Ubuntu.
>>
> 
> Yes, historically grub did case insensitive substring search. This
> probably is wrong, we should just take everything after boot number
> literally.

I see, like removing what you are about to add I guess.
The problem that I see is that efibootmgr output (even if --verbose
switch) it's not machine readable.

I guess efibootmgr itself would need an specific switch in order to
produce output suitable for scripts. Another option is include some of
the efibootmgr functionality/libraries into grub itself.

Maybe there's something on upstream's efibootmgr. Not a clue about that.
I have only checked Debian stretch's efibootmgr. I might ask about it in
debian-efi mailing list.

> ...
>> 1) First of all this matches all the line:
>>
>> if (!strcasestr (line, efi_distributor))
>> continue;
>>
>> That means that if you add a custom label which matches the efi distributor
>> then it gets removed. I think that's what happened to me. I would prefer
>> something more precise that would check the complete efi file path agains
>> e.g. EFI/vendor/ .
>>
>> 2) Then there's:
>>
>>       if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>>  || line[sizeof ("Boot") - 1] < '0'
>>  || line[sizeof ("Boot") - 1] > '9')
>> continue;
>>
>> which might be wrong because of 0 and 9 and maybe not because of the array
>> indexes.
>>
>> Let's go into details about that.
>>
>> 2.1) Boot0000 First entry
>> BootA000 Second entry
>>
>> Shouldn't the look for A to F hexadecimal letters too?
>>
> 
> Yes. Patches are welcome for both problems. Second one is actually bug
> fix so should be independent.
> 
>> And...

Well, I think just checking 0 to 9 in the first character is a good
compromise.

Some outputs have: BootCurrent . So 'BootC' can be found in e.g.
'BootC001' too. So that would be adding another problem because
'BootCurrent' would be considered as a right entry.

Just checking the first character leaves place for 16^3 = (2^4)^3
= 2 ^ (4 * 3 ) = 2 ^12 =  4096 .

That should be enough for most of the usecases.


>>
>> 2.2) line[sizeof ("Boot") - 1] < '0'
>>
>> Am I doing it right?
>>
>> sizeof ("Boot") = 4
>>
> 
> It is 5.
Ok, yes, sizeof is not length so... it shows what it takes to save it.
So... 4 bytes and the 'finish string byte' so that makes 5.


Well, I have finally decided not to put the full path to efi file and
only the basename of it. That will avoid custom entries being suddenly
removed by grub-install.

Thank you for your feedback.


adrian15
-- 
Support free software. Donate to Super Grub Disk. Apoya el software
libre. Dona a Super Grub Disk. http://www.supergrubdisk.org/donate/