Re: grub-install deleting long UEFI entries bug ?

2017-04-23 Thread 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.

> ...
> >
> > 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 ?

2017-04-23 Thread Andrei Borzenkov
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

2017-04-23 Thread Ralph Ronnquist
 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

2017-04-23 Thread Andrei Borzenkov
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

2017-04-23 Thread Bruce Dubbs

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 ?

2017-04-23 Thread 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.

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 ?

2017-04-23 Thread Andrei Borzenkov
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