Re: grub-install deleting long UEFI entries bug ?
2017-04-23 6:36 GMT+02:00 Andrei Borzenkov : > 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. > ... > > > > Maybe grub-install uses efibootmgr as an auxiliar tool and the problem > > is in Ubuntu's efibootmgr? > > > > Yes. > Yeah, I see it right in the source code. More to come. > > > It would be nice if someone could point us on where does grub-install > > handles the uefi boot entries so that we can take a deeper look into it. > > > > grub-core/osdep/unix/platform.c > Thank you. I've taken a look at: grub-core/osdep/unix/platform.c file (on 2.02-rc2 tag) and I have some comments about it. There's the function: grub_install_remove_efi_entries_by_distributor which has some interesting snippets: 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) Boot First entry BootA000 Second entry Shouldn't the look for A to F hexadecimal letters too? And... 2.2) line[sizeof ("Boot") - 1] < '0' Am I doing it right? sizeof ("Boot") = 4 So it's line [4 - 1] and therefore: line [3] . And as a consequence... line[3] = 't'. line[3] is always going to be 't' when the first OR condition is not true...so no need of the third or the fourth condition then. Or is there something about indexes or character size (unicode being 16bit?) that I am missing? Thank you. adrian15 -- Support free software. Donate to Super Grub Disk. Apoya el software libre. Dona a Super Grub Disk. http://www.supergrubdisk.org/donate/ . ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-install deleting long UEFI entries bug ?
23.04.2017 11:21, adrian15 adrian15 пишет: > 2017-04-23 6:36 GMT+02:00 Andrei Borzenkov : > >> 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. ... > 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) Boot 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... > > 2.2) line[sizeof ("Boot") - 1] < '0' > > Am I doing it right? > > sizeof ("Boot") = 4 > It is 5. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
grub2-common: grub.cfg gains wrong root settings for multi-OS system
From: ralph.ronnqu...@gmail.com Subject: grub2-common: grub.cfg gains wrong root settings for multi-OS system Package: grub2-common Version: 2.02~beta2-22+deb8u1 Severity: normal Dear Maintainer, I have set the system up as follows /dev/sda1 is a grub partition /dev/sdb1 is Devuan 1.0 RC, NETINST install /dev/sdc1 is Devuan 1.0 RC, DVD install /dev/sdd1 is Devuan 1.0 RC, CD install For all installs, I made it mount /dev/sda1 at /boot but only the first install formatted it. update-grub does collate all the OSs, but the final configuration has the same 'root=...' arguments for all boot options, instead of the different root options. And therefore that same OS will be booted regardless of which one I choose. Ralph. -- Package-specific info: *** BEGIN /proc/mounts /dev/sdb1 / ext4 rw,relatime,errors=remount-ro,data=ordered 0 0 /dev/sda1 /boot ext2 rw,relatime 0 0 *** END /proc/mounts *** BEGIN /boot/grub/grub.cfg # # DO NOT EDIT THIS FILE # # It is automatically generated by grub-mkconfig using templates # from /etc/grub.d and settings from /etc/default/grub # ### BEGIN /etc/grub.d/00_header ### if [ -s $prefix/grubenv ]; then set have_grubenv=true load_env fi if [ "${next_entry}" ] ; then set default="${next_entry}" set next_entry= save_env next_entry set boot_once=true else set default="0" fi if [ x"${feature_menuentry_id}" = xy ]; then menuentry_id_option="--id" else menuentry_id_option="" fi export menuentry_id_option if [ "${prev_saved_entry}" ]; then set saved_entry="${prev_saved_entry}" save_env saved_entry set prev_saved_entry= save_env prev_saved_entry set boot_once=true fi function savedefault { if [ -z "${boot_once}" ]; then saved_entry="${chosen}" save_env saved_entry fi } function load_video { if [ x$feature_all_video_module = xy ]; then insmod all_video else insmod efi_gop insmod efi_uga insmod ieee1275_fb insmod vbe insmod vga insmod video_bochs insmod video_cirrus fi } if [ x$feature_default_font_path = xy ] ; then font=unicode else insmod part_msdos insmod ext2 set root='hd1,msdos1' if [ x$feature_platform_search_hint = xy ]; then search --no-floppy --fs-uuid --set=root --hint-bios=hd1,msdos1 --hint-efi=hd1,msdos1 --hint-baremetal=ahci1,msdos1 9269015f-3aec-480e-8c8c-9f2fa3a71a10 else search --no-floppy --fs-uuid --set=root 9269015f-3aec-480e-8c8c-9f2fa3a71a10 fi font="/usr/share/grub/unicode.pf2" fi if loadfont $font ; then set gfxmode=auto load_video insmod gfxterm set locale_dir=$prefix/locale set lang=en_AU insmod gettext fi terminal_output gfxterm if [ "${recordfail}" = 1 ] ; then set timeout=-1 else if [ x$feature_timeout_style = xy ] ; then set timeout_style=menu set timeout=5 # Fallback normal timeout code in case the timeout_style feature is # unavailable. else set timeout=5 fi fi ### END /etc/grub.d/00_header ### ### BEGIN /etc/grub.d/05_debian_theme ### set menu_color_normal=cyan/blue set menu_color_highlight=white/blue ### END /etc/grub.d/05_debian_theme ### ### BEGIN /etc/grub.d/10_linux ### function gfxmode { set gfxpayload="${1}" } set linux_gfx_mode= export linux_gfx_mode menuentry 'Devuan GNU/Linux' --class devuan --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-simple-9269015f-3aec-480e-8c8c-9f2fa3a71a10' { load_video insmod gzio if [ x$grub_platform = xxen ]; then insmod xzio; insmod lzopio; fi insmod part_msdos insmod ext2 set root='hd0,msdos1' if [ x$feature_platform_search_hint = xy ]; then search --no-floppy --fs-uuid --set=root --hint-bios=hd0,msdos1 --hint-efi=hd0,msdos1 --hint-baremetal=ahci0,msdos1 143e4ede-df6e-46f7-956f-04a538d93881 else search --no-floppy --fs-uuid --set=root 143e4ede-df6e-46f7-956f-04a538d93881 fi echo'Loading Linux 3.16.0-4-amd64 ...' linux /vmlinuz-3.16.0-4-amd64 root=UUID=9269015f-3aec-480e-8c8c-9f2fa3a71a10 ro quiet echo'Loading initial ramdisk ...' initrd /initrd.img-3.16.0-4-amd64 } submenu 'Advanced options for Devuan GNU/Linux' $menuentry_id_option 'gnulinux-advanced-9269015f-3aec-480e-8c8c-9f2fa3a71a10' { menuentry 'Devuan GNU/Linux, with Linux 3.16.0-4-amd64' --class devuan --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-3.16.0-4-amd64-advanced-9269015f-3aec-480e-8c8c-9f2fa3a71a10' { load_video insmod gzio if [ x$grub_platform = xxen ]; then insmod xzio; insmod lzopio; fi insmod part_msdos insmod ext2 set root='hd0,msdos1' if [ x$feature_platform_search_hint = xy ]; then search --no-floppy --fs-uuid --set=root --hint-bios=hd0,msdos1 --hint-efi=hd0,msdos1 --hint-baremetal=ahci0,msdos1 143e4ed
Re: grub2-common: grub.cfg gains wrong root settings for multi-OS system
23.04.2017 08:43, Ralph Ronnquist пишет: > For all installs, I made it mount /dev/sda1 at /boot but only the > first install formatted it. Shared /boot never worked reliably. With or without grub. Sorry. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub2-common: grub.cfg gains wrong root settings for multi-OS system
Andrei Borzenkov wrote: 23.04.2017 08:43, Ralph Ronnquist пишет: For all installs, I made it mount /dev/sda1 at /boot but only the first install formatted it. Shared /boot never worked reliably. With or without grub. Sorry. It works perfectly if you don't use grub-mkconfig. For those using multiple kernels/partitions, grub.config can be quite simple to maintain with an editor. http://www.linuxfromscratch.org/lfs/view/stable/chapter08/grub.html That page shows a 7 line grub.cfg. For a new system you only need to add: menuentry "New title" { linux / root=/dev/ ro } An initrd line is only needed if you need an initial ram disk. You do need to keep a backup as distros always want to overwrite grub.cfg by running grub-mkconfig. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-install deleting long UEFI entries bug ?
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 : >> >>> 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) Boot 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/ ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-install deleting long UEFI entries bug ?
23.04.2017 23:33, 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 : >>> 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. > We are using plain "efibootmgr" without --verbose and output is variable name followed by either '*' or space followed by space followed by description if present. Unless description contains newline, it looks straightforward to parse. > I guess efibootmgr itself would need an specific switch in order to > produce output suitable for scripts. It would need somehow escape newline > Another option is include some of > the efibootmgr functionality/libraries into grub itself. > Yes, I have half-done patch but as I see now it is probably not really needed unless we actually have case of description containing newlines. >>> >>> 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) Boot 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. > This simply means that we have to check for exactly 4 hexadecimal numbers, not shortcut this. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel