Re: [PATCH] util/grub.d/30_os-prober.in: Fix GRUB_OS_PROBER_SKIP_LIST for non-EFI

From: Pascal Hambourg <pascal@plouf.fr.eu.org>
To: The development of GNU GRUB <grub-devel@gnu.org>,
	Glenn Washburn <development@efficientek.com>
Cc: Daniel Kiper <dkiper@net-space.pl>
Subject: Re: [PATCH] util/grub.d/30_os-prober.in: Fix GRUB_OS_PROBER_SKIP_LIST for non-EFI
Date: Wed, 1 May 2024 22:56:27 +0200	[thread overview]
Message-ID: <c31461fb-d6b2-4be0-a8ef-908ed08b2864@plouf.fr.eu.org> (raw)
In-Reply-To: <20240427013235.0e6ab6de@crass-HP-ZBook-15-G2>

On 27/04/2024 at 08:32, Glenn Washburn wrote:
> On Fri, 19 Jan 2024 11:28:55 +0100
> Pascal Hambourg <pascal@plouf.fr.eu.org> wrote:
> 
>> GRUB documentation states:
>>
>> 'GRUB_OS_PROBER_SKIP_LIST'
>>    List of space-separated FS UUIDs of filesystems to be ignored from
>>    os-prober output. For efi chainloaders it's <UUID>@<EFI FILE>
>>
>> But the actual behaviour does not match this description. Setting
>>
>>    GRUB_OS_PROBER_SKIP_LIST="<UUID>"
>>
>> does not ignore non-EFI boot loaders detected on filesystem <UUID>.
>> In order to skip non-EFI boot loaders, you must set
>>
>>    GRUB_OS_PROBER_SKIP_LIST="<UUID>@<DEVICE>"
>>
>> which is both absurd (UUID and device are redundant) and wrong
>> (device names such as /dev/sd* may not be persistent across boots).
>>
>> This patch fixes the detection of "@<EFI FILE>" in the device string
>> reported by os-prober.
>>
>> Fixes: 55e706c9 (Add GRUB_OS_PROBER_SKIP_LIST to selectively skipping systems)
>>
>> Note: the UUID matching regex uses word boundaries '\b' but '@' is
>> not a word character. As a consequence, setting
>>
>>    GRUB_OS_PROBER_SKIP_LIST="<UUID>@<ANYTHING>"
>>
>> will ignore non-EFI boot loaders detected on filesystem <UUID> even
>> though the goal is to ignore a given EFI boot loader only.
>> However I think this is desirable to preserve the behaviour of
>> existing setups which use GRUB_OS_PROBER_SKIP_LIST="<UUID>@<DEVICE>".

Additionally, '-' may be present in UUIDs and is not a word character 
either, so the regex may also match UUID substrings instead of whole 
UUIDs only. For example a FAT UUID may match an ext4 UUID substring.

> It would be good to have this in a comment in an appropriate place in
> the code. I know of no complaints about this,

I found these in grub lists:
<https://lists.gnu.org/archive/html/help-grub/2018-11/msg00005.html>
<https://lists.gnu.org/archive/html/bug-grub/2022-12/msg00002.html>

And in a few forum threads:
<https://www.linuxquestions.org/questions/linux-desktop-74/solved-grub-detecting-2-windows10-only-one-really-works-4175678998/>
<https://forum.manjaro.org/t/after-installing-manjaro-xfce-there-are-two-options-for-boot-in-my-uefi-one-is-uefi-os-and-another-one-is-my-ssd-which-one-should-i-choose/12592/15>
<https://bbs.archlinux.org/viewtopic.php?pid=2075691#p2075691>
<https://superuser.com/questions/237565/how-to-hide-recovery-partition-from-grub2>
<https://askubuntu.com/questions/453038/update-grub-ignoring-some-options-in-etc-default-grub>

I guess some people have come to believe the current behaviour is 
normal, for example 
<https://wiki.phobos-net.fr/doku.php/sysadmin/grub2/supprimer_options_demarrage>

> but I think it would be
> good to work on transitioning to what should be done.

What is the "correct behavior" regarding items containing '@' ?
Is it acceptable that "<UUID>@<EFIPATH>" skips non-EFI boot loaders, as 
implemented by this patch ?
Or should only "<UUID>" and "<UUID>@/dev/*" skip non-EFI boot loaders ?

Is is acceptable to skip a boot loader if its UUID is only a substring 
of a UUID in GRUB_OS_PROBER_SKIP_LIST (which is highly unlikely) ?

> Perhaps the
> correct behavior should be available with a envvar and wait a full
> release to to enable by default giving people time to transition.

What do you mean by "available with a envvar" ?
- Define a new variable such as GRUB_OS_PROBER_UUID_SKIP_LIST whose 
processing implements the correct behaviour and preserve 
GRUB_OS_PROBER_SKIP_LIST current processing ?
- Define a new variable such as GRUB_OS_PROBER_SKIP_LIST_FIX whose value 
affects the processing of GRUB_OS_PROBER_SKIP_LIST ?
- Something else ?

>> Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>
>> ---
>>   util/grub.d/30_os-prober.in | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
>> index e9e217208..a24c01334 100644
>> --- a/util/grub.d/30_os-prober.in
>> +++ b/util/grub.d/30_os-prober.in
>> @@ -125,7 +125,7 @@ for OS in ${OSPROBED} ; do
>>     if UUID="`${grub_probe} --target=fs_uuid --device ${DEVICE%@*}`"; then
>>       EXPUUID="$UUID"
>>   
>> -    if [ x"${DEVICE#*@}" != x ] ; then
>> +    if [ x"${DEVICE%@*}" != x"${DEVICE}" ] ; then
>>         EXPUUID="${EXPUUID}@${DEVICE#*@}"
>>       fi

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel