Bugzilla Bug 218599

In current grub ebuilds, there is a section in setup_boot_dir that greps most
commands from grub.conf, the grub boot menu file, and unconditionally executes
them with the grub shell. I consider this dangerous and harmful.

Suppose you have a menu entry to setup grub on a floppy disk, and while
emerging grub you accidentially have a floppy with some valuable data inserted?
Or maybe your device maps don't fit, you have a menu entry for installing grub,
and consequently grub overwrites the boot sector of the wrong drive?

You might allow for automatic reinstallation of grub on update, but for this,
you should introduce a new config file, and not reuse grub.conf.

You have a point but there is a DONT_MOUNT_BOOT variable. When it is non-zero,
setup_boot_dir function will not be executed.

echo DONT_MOUNT_BOOT=\"yes\" >> /etc/make.conf

and you will be fine, i think

Panagiotis (pchrist)

(In reply to comment #1)
> You have a point but there is a DONT_MOUNT_BOOT variable. When it is non-zero,
> setup_boot_dir function will not be executed.

Interesting point. Generally, I'd think not running grub with these commands
should probably be the default, to play it safe. Personally, I would want to
have the grub files copied to the boot directory, which would be prevented as
well by setting that variable.

But you could combine that variable and the auto-install commands file in an
interesting way: introduce some "${ROOT}"/etc/grub-install.conf or
"${ROOT}"/etc/grub.install or whatever file, and if it exists, mount boot and
run that file thorugh grub shell, and if it doesn't, log some useful message
instead, mentioning that possibility.

The danger of not running the setup_boot_dir stuff is that if grub is updated,
and stagefiles change, the /boot copy won't get updated, which in some cases
can lead to an unbootable box.

If we do change this, we need to make sure that the handbook docs match, and
that every user is aware of it, so that they always run 'emerge --config
sys-boot/grub' after an update.

(In reply to comment #3)
> The danger of not running the setup_boot_dir stuff is that if grub is updated,
> and stagefiles change, the /boot copy won't get updated, which in some cases
> can lead to an unbootable box.

If you don't run this whole setup_boot_dir stuff, then the stages won't be
overridden, but that just amounts to DONT_MOUNT_BOOT=yes, I guess. So the
problem is copying the stages (in setup_boot_dir) but not installing them. The
current approach will not always help against this, as my own bug #98768
prooves. I have no setup or install command in my boot menu. By the way, that's
a condition that should be pretty easy to check, and might be useful for
migration.

> If we do change this, we need to make sure that the handbook docs match, and
> that every user is aware of it, so that they always run 'emerge --config
> sys-boot/grub' after an update.

How about this:
1. Provide a install config file as part of the ebuild
2. Have that file contain "setup (hd0)" and/or "setup (hd0,0)" as comments,
along with description
3. Have the ebuild check that file contains either setup or install outside a
comment, if so run them (stripping all comments), otherwise ewarn the user to
update the file or install grub manually, including copying of the stage files.
4. Update the manual to mention this file

Migration path: file is default, admin gets ewarn, updates file, runs emerge
--config sys-boot/grub as requested by ewarn, and everything should be fine.

New users path: users read manual, learn about file, edit it, run emerge
--config sys-boot/grub as (then) described in manual, and everything should be
fine.

Update path: ebuild checks file, notices it's been worked on, automatically
copies stages and executes commands from config file, thus updating
automatically.

I'd prefer to keep the behavior of grub and what's mentioned in the handbooks
as-is. It's worked for all our users up to this point. This bug describes a
theoretical edge case.

If you leave in a floppy you didn't want overwritten, well, tough luck. Same
goes for rm -rf / -- changing documentation to force an additional step that's
never been needed increases the chances of user error, not decreases.

One of the nice things about using grub is that you don't have to touch it
after an update. Forcing --config after updating makes it little better than
lilo.

At a quick glance, executing all lines concerning "root" or "install" in
grub.conf doesn't look like an excellent idea, but forcing the user to perform
some manuall steps wouldn't be really user-friendly, either.

I guess that executing all commands before first "title" line won't work in
following case:

title kernel
root (hd6,0)
kernel /linux

(In reply to comment #5)
> I'd prefer to keep the behavior of grub and what's mentioned in the handbooks
> as-is. It's worked for all our users up to this point.

How so?
http://www.gentoo.org/doc/en/handbook/handbook-x86.xml?part=1&chap=10#doc_chap2
doesn't give any indication that install commands should be included in the
menu. It does, on the other hand, describe ways to install grub manually on
first run. So I'd be surprised if automatic reinstalation would work for even a
majority of users.

> This bug describes a theoretical edge case.
> If you leave in a floppy you didn't want overwritten, well, tough luck. Same
> goes for rm -rf /

That's an invalid comparison. If I type the rm command, I can expect it to do
what it will do. If I run an emerge -u, I will only expect it to update my
software, not to wipe my floppies. So if the floppy get's wiped, I'd not blame
the users here, but the ebuild.

> One of the nice things about using grub is that you don't have to touch it
> after an update.

On the contrary, I have to make damn sure to configure it at least afer every
second update, or I'll get bit by bug #98768.

> Forcing --config after updating makes it little better than lilo.

I wasn't suggesting that. If users create a file describing how to install
grub, then the ebuild can and should do so automatically. That's waht I
described as "Update path" in comment #4. I merely don't want these commands in
the boot menu, as the install step doesn't belong in the menu, and the menu
commands don't belong with the install process.

(In reply to comment #6)
> At a quick glance, executing all lines concerning "root" or "install" in
> grub.conf doesn't look like an excellent idea, but forcing the user to perform
> some manuall steps wouldn't be really user-friendly, either.

You definitely want manual steps at first install, at least show the users the
defaults you would use and ask him to confirm. Otherwise Gentoo would be worse
than Windows. If you remember these steps, you can automatically execute them
on update.

> I guess that executing all commands before first "title" line won't work in
> following case:

Before the first title? The current ebuild would execute commands from al
places, stripping title among others.

> title kernel
> root (hd6,0)
> kernel /linux

Would do exactly nothing, as there is neither setup nor install command. That's
pretty much the most simple scenario where automatic update will fail even
without a floppy in the drive. I'd consider it pretty common.

here's a generalization that i'm sure the docs team will agree with: people
dont read.  the updating of the boot loader needs to be automatic and safe.  if
the only unsafe condition you can propose is the highly uncommon (and unlikely)
scenario where people have lines in their grub.conf to boot/setup a floppy
disc, and they happen to have a floppy disc in their drive at the time of the
emerge, and the mbr may get clobbered, then that's tough love for you.

(In reply to comment #8)
> here's a generalization that i'm sure the docs team will agree with: people
> dont read.  the updating of the boot loader needs to be automatic and safe.  if
> the only unsafe condition you can propose is the highly uncommon (and unlikely)
> scenario where people have lines in their grub.conf to boot/setup a floppy
> disc, and they happen to have a floppy disc in their drive at the time of the
> emerge, and the mbr may get clobbered, then that's tough love for you.

Exactly. Un-CCing the GDP, as there's nothing for us to do here.

Created an attachment (id=150501) [details]
Sane automatic installation

This is my first shot at a patch to illustrate my suggestions.
Let's see what's new in here.

1. introduced grub.install, with full path in $instcfg. For the time being, I
placed that in /etc, but one might as well put it in $dir/grub. Up to you.

2. Demoted warning about moved menu.lst, because there are more important notes
in the new ebuild, and calling every message important is kind of pointless.

3. Create grub.install if it does not exist. Right now, its contents is
included in setup_boot_dir within the ebuild, but it might be moved to
$FILESDIR, and it might be installed in the install phase and thus be merged
with full config protection. This would make automatic cutsomization of that
file difficult, though.

4. If grub.conf exists and contains setup or install commands, these are
appended to grub.install. The sed script basically takes all menu entries with
a setup or install in them, so we don't get all those commands used to actually
boot some system. From the general commands before the first title, only the
root command is included, I'd guess that should be enough.
If you are worried about this whole sed script, you may use the old egrep
command instead, but I'd prefer the sed, as it is more restrictive in what it
takes, and thus closer to my original intent, while it still should work.

5. If there are setup instructions, either freshly extracted from grub.conf, or
installed from a previous build and probably edited by the admin, then execute
these commands in order to automatically update grub, even its stage 1.

6. Do some error checking. The return calue from the grub shell seems to be no
indication of errors. At least for me, a "setup (hd0,0)" without prior root
caused an error and still resulted in a zero return value. Therefore I
redirected the whole output from the grub shell to a temporary file, and
grepped for error messages. The return value would still be useful in case of
signals.

7. If there are no automatic update commands, tell the user to update manually.
Also, if this is an update, warn him especially that failure to update might
result in an unbootable system - my favorite bug #98768. I changed the wording
somewhat, as you don't have to install into the MBR but can install into a
partiton boot sector, and what stage2 gets executed depends on if you have a
stage1.5, i.e. if you refer to the stage2 by block list or file name.

On the whole, this should give you
- No warnings or other log entries for successful automatic updates
- More accurate messages when there is something to be done
- More information when an error occurs
- An independent file for the install instructions
- Automatic migration, expected without lost functionality or extra steps
- A pretty sane subset of commands to be migrated

Did some testing, with and without grub.install present, with and without a
setup command in my grub.conf. So far, everything looks good. Comments?

I'm going to consider this later for r6 or r7.
Just because I don't have time at the moment (going to Africa, back in 2
weeks).

My comments about this:

- Updated my grub today
- Didn't have any problems, because i don't have a setup-command in my
grub.conf (so grub actually does nothing, right?)
- So i have added a proper root- and setup-command to my grub.conf to see what
happens.
- I was REALLY pissed to see, that i see nothing, because grub output is piped
to /dev/null and nobody checks if grub has failed or not or if there was a
setup-command or not.
- Is there any standard way to store somewhere, where grub should be installed?
So there is a program called grub-install, but it doesn't have a config file
and doesn't use grub.conf. Instead, it takes a device name as a parameter.
Where is a program/script, that actually uses the configuration from grub.conf
to (re)install grub? (If i configured it somewhere, i'd like to be abled to
easily execute it)
- In which device's bootsector grub is installed, might be crucial for future
support of hibernate, altering the boot-sector/MBR before going to suspend to
disk. Any clue, what hibernate might expect in the future?
- What if a grub.conf/grub.install or whatever exists, but i'm actually
currently using LILO? The ebuild should check, if grub is really installed.
(read 512 from the device, grep for GRUB to test, whether GRUB is installed.
Unfortunatly, i don't know a good tool, to analyse the boot sector/MBR and grub
doesn't offer any, i think). Of course, the device's name is needed, but
grub.conf only supplies something like (hd0,0) which is hard to map back to a
device name while grub-install takes a good old plain device name like in
"grub-install /dev/sda2"
- It's hard to have a good sane system for managing boot loaders. Don't do
anything or do something really good. The current sollution is in between and
therefor error prone.

Just my 50 cent.

(In reply to comment #12)
> - In which device's bootsector grub is installed, might be crucial for future
> support of hibernate, altering the boot-sector/MBR before going to suspend to
> disk. Any clue, what hibernate might expect in the future?

I would assume that this would use grub-set-default or maybe scripted
manipulation of /boot/grub/default rather than any low-level modification of
the boot sector. But as hibernation never worked for me (due to binary
drivers), I might be wrong.

> - What if a grub.conf/grub.install or whatever exists, but i'm actually
> currently using LILO?

My suggested patch would log a message if grub.install was comments only and
execute the setup/install comments if there were any, not caring about lilo. I
would expect people to only edit grub.install when they are using grub. As
there are some ugly guesses involved with checking for GRUB, just as you
pointed out, I would rather not do this automatically.

> Of course, the device's name is needed, but
> grub.conf only supplies something like (hd0,0) which is hard to map back to a
> device name while grub-install takes a good old plain device name like in
> "grub-install /dev/sda2"

One might use /boot/grub/device.map for the drive, from where access to the
partitions should be easier at least. I don't know how far we can trust the
contents of that file, but as it is used by the grub shell as well, we'd at
least be consistent with the executed setup commands. One can have a look at
grub-install to see how this file can be (re)created when it's not present.

> Just my 50 cent.

Thanks!

grub.conf.install implemented.