(no subject)

[Mathieu Trudel-Lapierre]

I've been playing with fixing Ubuntu bug 1097570; trying to debug a script we
use for the EFI images which detects where to boot from based on the location
of a .disk/info file.

Turns out this mostly works, except for the fact that grub appears to always
list devices in a specific order, which interferes with the detection if more
than one device contains that file.

I think I can make this work appropriately provided we:
  - Use hints for search --file;
  - Have some value to pass to --hint= designating the boot drive.

Included is a patch that adds a "bootdrive" variable alongside cmdpath; which
contains just the drive designation for the boot device, to be used as such:

search --set=root --hint=$bootdrive --file .disk/info

I'd much appreciate review on that patch :)

[PATCH] Set a bootdrive environment variable.

[Mathieu Trudel-Lapierre]

---
 grub-core/kern/main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 9cad0c4..8352b6e 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -130,6 +130,8 @@ grub_set_prefix_and_root (void)
   if (fwdevice)
     {
       char *cmdpath;
+      char *bootdrive = NULL;
+      char *cptr;
 
       cmdpath = grub_xasprintf ("(%s)%s", fwdevice, fwpath ? : "");
       if (cmdpath)
@@ -138,6 +140,19 @@ grub_set_prefix_and_root (void)
 	  grub_env_export ("cmdpath");
 	  grub_free (cmdpath);
 	}
+
+      cptr = grub_strrchr (fwdevice, ',');
+      if (cptr)
+        {
+          bootdrive = grub_strndup (fwdevice, cptr - fwdevice);
+          cptr++;
+        }
+      if (bootdrive)
+        {
+          grub_env_set ("bootdrive", bootdrive);
+          grub_env_export ("bootdrive");
+          grub_free (bootdrive);
+        }
     }
 
   if (prefix)
-- 
2.1.4

Re: Set a bootdrive environment variable.

[Andrei Borzenkov]

В Tue, 26 May 2015 12:46:07 -0400
Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:

> I've been playing with fixing Ubuntu bug 1097570; trying to debug a script we
> use for the EFI images which detects where to boot from based on the location
> of a .disk/info file.
> 
> Turns out this mostly works, except for the fact that grub appears to always
> list devices in a specific order, which interferes with the detection if more
> than one device contains that file.
> 
> I think I can make this work appropriately provided we:
>   - Use hints for search --file;
>   - Have some value to pass to --hint= designating the boot drive.
> 

If you always want to use the same device GRUB was booted from why do
you need to search for anything at all?

> Included is a patch that adds a "bootdrive" variable alongside cmdpath; which
> contains just the drive designation for the boot device, to be used as such:
> 
> search --set=root --hint=$bootdrive --file .disk/info
> 
> I'd much appreciate review on that patch :)
> 

You can extract it from $cmdpath using regexp already. In retrospect
having separate variables for device and path would be more flexible;
but $drive is not appropriate name as it can refer to network as well.
Something like $grub_bootdev and $grub_bootpath.

Re: Set a bootdrive environment variable.

[Mathieu Trudel-Lapierre]

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

On Wed, May 27, 2015 at 12:04 AM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> В Tue, 26 May 2015 12:46:07 -0400
> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
>
> > I've been playing with fixing Ubuntu bug 1097570; trying to debug a
> script we
> > use for the EFI images which detects where to boot from based on the
> location
> > of a .disk/info file.
> >
> > Turns out this mostly works, except for the fact that grub appears to
> always
> > list devices in a specific order, which interferes with the detection if
> more
> > than one device contains that file.
> >
> > I think I can make this work appropriately provided we:
> >   - Use hints for search --file;
> >   - Have some value to pass to --hint= designating the boot drive.
> >
>
> If you always want to use the same device GRUB was booted from why do
> you need to search for anything at all?
>

My understanding from why the search was used is that $root may not have
been set for all types of devices (i.e. CDs), and the same built grub
images (for EFI) were used on bootable USB and CDs since they are both
built from a single ISO.

In retrospect, having a properly set $cmdpath (or $grub_bootdev or
whatever), probably makes this unnecessary.


> > Included is a patch that adds a "bootdrive" variable alongside cmdpath;
> which
> > contains just the drive designation for the boot device, to be used as
> such:
> >
> > search --set=root --hint=$bootdrive --file .disk/info
> >
> > I'd much appreciate review on that patch :)
> >
>
> You can extract it from $cmdpath using regexp already. In retrospect
> having separate variables for device and path would be more flexible;
> but $drive is not appropriate name as it can refer to network as well.
> Something like $grub_bootdev and $grub_bootpath.
>

That was one of the reasons I wanted review on it, so thanks!

I'm happy to change it to $grub_bootdev or whatever else if we feel having
separate variable for this in indeed a good idea (and I'll add what's
needed to get $grub_bootpath).


Mathieu Trudel-Lapierre <mathieu.tl@gmail.com>
Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93

[-- Attachment #2: Type: text/html, Size: 3221 bytes --]

Re: Set a bootdrive environment variable.

[Andrei Borzenkov]

В Wed, 27 May 2015 08:55:09 -0400
Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:

> On Wed, May 27, 2015 at 12:04 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> 
> > В Tue, 26 May 2015 12:46:07 -0400
> > Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
> >
> > > I've been playing with fixing Ubuntu bug 1097570; trying to debug a
> > script we
> > > use for the EFI images which detects where to boot from based on the
> > location
> > > of a .disk/info file.
> > >
> > > Turns out this mostly works, except for the fact that grub appears to
> > always
> > > list devices in a specific order, which interferes with the detection if
> > more
> > > than one device contains that file.
> > >
> > > I think I can make this work appropriately provided we:
> > >   - Use hints for search --file;
> > >   - Have some value to pass to --hint= designating the boot drive.
> > >
> >
> > If you always want to use the same device GRUB was booted from why do
> > you need to search for anything at all?
> >
> 
> My understanding from why the search was used is that $root may not have
> been set for all types of devices (i.e. CDs), and the same built grub
> images (for EFI) were used on bootable USB and CDs since they are both
> built from a single ISO.
> 

That depends on your image. If image defines prefix and drive and/or
partition is missing (e.g. (,msdos1)) missing parts are derived from
whatever firmware provides (effectively what ends up in $cmdpath) and
should be set for any boot media.

Although I just hit a system (Fujitsu Lifebook S761) which passes bogus
device path to image that does not match any existing handle, so both
$cmdpath and $root are not set.

> In retrospect, having a properly set $cmdpath (or $grub_bootdev or
> whatever), probably makes this unnecessary.
> 
> 
> > > Included is a patch that adds a "bootdrive" variable alongside cmdpath;
> > which
> > > contains just the drive designation for the boot device, to be used as
> > such:
> > >
> > > search --set=root --hint=$bootdrive --file .disk/info
> > >
> > > I'd much appreciate review on that patch :)
> > >
> >
> > You can extract it from $cmdpath using regexp already. In retrospect
> > having separate variables for device and path would be more flexible;
> > but $drive is not appropriate name as it can refer to network as well.
> > Something like $grub_bootdev and $grub_bootpath.
> >
> 
> That was one of the reasons I wanted review on it, so thanks!
> 
> I'm happy to change it to $grub_bootdev or whatever else if we feel having
> separate variable for this in indeed a good idea (and I'll add what's
> needed to get $grub_bootpath).
>

The problem is that $cmdpath had been around for long enough. If
$cmdpath is used only after loading normal (that are cases I'm aware
of) we can make it compatibility variable set by normal without causing
more core bloat.

> 
> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com>
> Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
> 4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93

Re: Set a bootdrive environment variable.

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 3515 bytes --]

What's the problem with extracting this from $prefix ? This seems to be
a better match if what you want is location of GRUB.
On 31.05.2015 08:42, Andrei Borzenkov wrote:
> В Wed, 27 May 2015 08:55:09 -0400
> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
> 
>> On Wed, May 27, 2015 at 12:04 AM, Andrei Borzenkov <arvidjaar@gmail.com>
>> wrote:
>>
>>> В Tue, 26 May 2015 12:46:07 -0400
>>> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
>>>
>>>> I've been playing with fixing Ubuntu bug 1097570; trying to debug a
>>> script we
>>>> use for the EFI images which detects where to boot from based on the
>>> location
>>>> of a .disk/info file.
>>>>
>>>> Turns out this mostly works, except for the fact that grub appears to
>>> always
>>>> list devices in a specific order, which interferes with the detection if
>>> more
>>>> than one device contains that file.
>>>>
>>>> I think I can make this work appropriately provided we:
>>>>   - Use hints for search --file;
>>>>   - Have some value to pass to --hint= designating the boot drive.
>>>>
>>>
>>> If you always want to use the same device GRUB was booted from why do
>>> you need to search for anything at all?
>>>
>>
>> My understanding from why the search was used is that $root may not have
>> been set for all types of devices (i.e. CDs), and the same built grub
>> images (for EFI) were used on bootable USB and CDs since they are both
>> built from a single ISO.
>>
> 
> That depends on your image. If image defines prefix and drive and/or
> partition is missing (e.g. (,msdos1)) missing parts are derived from
> whatever firmware provides (effectively what ends up in $cmdpath) and
> should be set for any boot media.
> 
> Although I just hit a system (Fujitsu Lifebook S761) which passes bogus
> device path to image that does not match any existing handle, so both
> $cmdpath and $root are not set.
> 
>> In retrospect, having a properly set $cmdpath (or $grub_bootdev or
>> whatever), probably makes this unnecessary.
>>
>>
>>>> Included is a patch that adds a "bootdrive" variable alongside cmdpath;
>>> which
>>>> contains just the drive designation for the boot device, to be used as
>>> such:
>>>>
>>>> search --set=root --hint=$bootdrive --file .disk/info
>>>>
>>>> I'd much appreciate review on that patch :)
>>>>
>>>
>>> You can extract it from $cmdpath using regexp already. In retrospect
>>> having separate variables for device and path would be more flexible;
>>> but $drive is not appropriate name as it can refer to network as well.
>>> Something like $grub_bootdev and $grub_bootpath.
>>>
>>
>> That was one of the reasons I wanted review on it, so thanks!
>>
>> I'm happy to change it to $grub_bootdev or whatever else if we feel having
>> separate variable for this in indeed a good idea (and I'll add what's
>> needed to get $grub_bootpath).
>>
> 
> The problem is that $cmdpath had been around for long enough. If
> $cmdpath is used only after loading normal (that are cases I'm aware
> of) we can make it compatibility variable set by normal without causing
> more core bloat.
> 
>>
>> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com>
>> Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
>> 4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: Set a bootdrive environment variable.

[Andrei Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 4043 bytes --]

В Thu, 04 Jun 2015 17:40:01 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> What's the problem with extracting this from $prefix ? This seems to be
> a better match if what you want is location of GRUB.

The original problem was that this information was needed to set
$prefix so it cannot be extracted from it :)

It can be extracted from $cmdpath but then it requires regexp in early
config and feels backwards in general.

> On 31.05.2015 08:42, Andrei Borzenkov wrote:
> > В Wed, 27 May 2015 08:55:09 -0400
> > Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
> > 
> >> On Wed, May 27, 2015 at 12:04 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> >> wrote:
> >>
> >>> В Tue, 26 May 2015 12:46:07 -0400
> >>> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:
> >>>
> >>>> I've been playing with fixing Ubuntu bug 1097570; trying to debug a
> >>> script we
> >>>> use for the EFI images which detects where to boot from based on the
> >>> location
> >>>> of a .disk/info file.
> >>>>
> >>>> Turns out this mostly works, except for the fact that grub appears to
> >>> always
> >>>> list devices in a specific order, which interferes with the detection if
> >>> more
> >>>> than one device contains that file.
> >>>>
> >>>> I think I can make this work appropriately provided we:
> >>>>   - Use hints for search --file;
> >>>>   - Have some value to pass to --hint= designating the boot drive.
> >>>>
> >>>
> >>> If you always want to use the same device GRUB was booted from why do
> >>> you need to search for anything at all?
> >>>
> >>
> >> My understanding from why the search was used is that $root may not have
> >> been set for all types of devices (i.e. CDs), and the same built grub
> >> images (for EFI) were used on bootable USB and CDs since they are both
> >> built from a single ISO.
> >>
> > 
> > That depends on your image. If image defines prefix and drive and/or
> > partition is missing (e.g. (,msdos1)) missing parts are derived from
> > whatever firmware provides (effectively what ends up in $cmdpath) and
> > should be set for any boot media.
> > 
> > Although I just hit a system (Fujitsu Lifebook S761) which passes bogus
> > device path to image that does not match any existing handle, so both
> > $cmdpath and $root are not set.
> > 
> >> In retrospect, having a properly set $cmdpath (or $grub_bootdev or
> >> whatever), probably makes this unnecessary.
> >>
> >>
> >>>> Included is a patch that adds a "bootdrive" variable alongside cmdpath;
> >>> which
> >>>> contains just the drive designation for the boot device, to be used as
> >>> such:
> >>>>
> >>>> search --set=root --hint=$bootdrive --file .disk/info
> >>>>
> >>>> I'd much appreciate review on that patch :)
> >>>>
> >>>
> >>> You can extract it from $cmdpath using regexp already. In retrospect
> >>> having separate variables for device and path would be more flexible;
> >>> but $drive is not appropriate name as it can refer to network as well.
> >>> Something like $grub_bootdev and $grub_bootpath.
> >>>
> >>
> >> That was one of the reasons I wanted review on it, so thanks!
> >>
> >> I'm happy to change it to $grub_bootdev or whatever else if we feel having
> >> separate variable for this in indeed a good idea (and I'll add what's
> >> needed to get $grub_bootpath).
> >>
> > 
> > The problem is that $cmdpath had been around for long enough. If
> > $cmdpath is used only after loading normal (that are cases I'm aware
> > of) we can make it compatibility variable set by normal without causing
> > more core bloat.
> > 
> >>
> >> Mathieu Trudel-Lapierre <mathieu.tl@gmail.com>
> >> Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
> >> 4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Set a bootdrive environment variable.

[Mathieu Trudel-Lapierre]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On Sun, May 31, 2015 at 2:42 AM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

>
> The problem is that $cmdpath had been around for long enough. If
> $cmdpath is used only after loading normal (that are cases I'm aware
> of) we can make it compatibility variable set by normal without causing
> more core bloat.
>

I'm not finding any uses of cmdpath in git, and when I went to dig in
RH/Fedora packaging, it looks like there is already a patch that appears to
rename it and change its meaning [1], along with two other related patches.

You mention you know of cases where it's used after loading normal, could
you please point me to them?

I'd like to try to come up with a patch that will work for everyone. FWIW,
my requirement right now is only to have some way (aside from regexp, since
we're concerned running regexp in the EFI context could increase attack
exposure) to get just the GRUB boot device (ie. cd0, hd0, etc. without
partition information), so that ISO images could boot and load a generic
grub.cfg that doesn't need to encode a filesystem UUID or some other
identifier.

[1]
http://pkgs.fedoraproject.org/cgit/grub2.git/tree/0090-Add-fw_path-variable-revised.patch

Kind regards,

/ Matt

[-- Attachment #2: Type: text/html, Size: 1769 bytes --]

Re: Set a bootdrive environment variable.

[Andrei Borzenkov]

В Mon, 6 Jul 2015 15:44:44 -0400
Mathieu Trudel-Lapierre <mathieu.tl@gmail.com> пишет:

> On Sun, May 31, 2015 at 2:42 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> 
> >
> > The problem is that $cmdpath had been around for long enough. If
> > $cmdpath is used only after loading normal (that are cases I'm aware
> > of) we can make it compatibility variable set by normal without causing
> > more core bloat.
> >
> 
> I'm not finding any uses of cmdpath in git, and when I went to dig in
> RH/Fedora packaging, it looks like there is already a patch that appears to
> rename it and change its meaning [1], along with two other related patches.
> 

Well, it is up to RH. Moreover, the real problem as indicated in rhbz is

--><--
The issue comes from the fact that the original patch does not clear the 
grub_errno in case the grub.cfg is not found in the bootlocation. 
--><--

and it is unrelated to how variable is named. I have patch for it
somewhere, I hit this issue as well. Another issue is missing initial
'/' which is fixed in grub in the meantime as well.

> You mention you know of cases where it's used after loading normal, could
> you please point me to them?
> 

I can think of some use cases and we never know how creative people
become in between.

> I'd like to try to come up with a patch that will work for everyone. FWIW,
> my requirement right now is only to have some way (aside from regexp, since
> we're concerned running regexp in the EFI context could increase attack
> exposure) to get just the GRUB boot device (ie. cd0, hd0, etc. without
> partition information), so that ISO images could boot and load a generic
> grub.cfg that doesn't need to encode a filesystem UUID or some other
> identifier.
> 

If you set initial prefix in grub.efi to '()', grub will strip
partition part when setting $root; look at what grub-mkrescue does. Is
it good enough for your purpose?


> [1]
> http://pkgs.fedoraproject.org/cgit/grub2.git/tree/0090-Add-fw_path-variable-revised.patch
> 
> Kind regards,
> 
> / Matt