search

voxpupuli / puppet-augeasproviders_grub

Augeas-based grub type and providers
Apache License 2.0
9 stars 33 forks source link

v4.0.0: Standard error of `grub-mkconfig` written to `grub.cfg` #74

Closed kenyon closed 1 year ago

kenyon commented 1 year ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

Use kernel_parameter and have it make a change to /etc/default/grub like

kernel_parameter { 'audit':
  ensure => present,
  value  => 1,
}

What are you seeing

/boot/grub/grub.cfg is created by grub-mkconfig, but the standard error (stderr) of this command is included along with the standard output, resulting in an invalid grub.cfg, possibly causing boot failure depending on where the bogus lines end up in grub.cfg. The standard error looks like this:

Generating grub configuration file ...
Found linux image: /boot/vmlinuz-6.0.0-0.deb11.6-amd64
Found initrd image: /boot/initrd.img-6.0.0-0.deb11.6-amd64
Found linux image: /boot/vmlinuz-6.0.0-0.deb11.2-amd64
Found initrd image: /boot/initrd.img-6.0.0-0.deb11.2-amd64
Warning: os-prober will be executed to detect other bootable partitions.
Its output will be used to detect bootable binaries on them and create new boot entries.
done

What behaviour did you expect instead

Only the standard output (stdout) of grub-mkconfig should be written to grub.cfg. This is how this module works in version 3.2.0.

Any additional information you'd like to impart

I think the bug was introduced by one of these commits:

bigon commented 1 year ago

Hello,

Any news on this?

If this is really causing boot failure, that should be fixed quickly I guess

gcoxmoz commented 1 year ago

The bug came in from commit 63c2c4bc085baebb7b29e61257a86ba7946913dd from #64. Before this, kernel_parameter/grub2.rb did mkconfig "-o", c : "run mkconfig and have it send its results to a file variable-named c", and "stderr" would just be ignored since it would appear but not be part of the -o output. After this commit landed, the effective call became PuppetX::AugeasprovidersGrub::Util.grub2_mkconfig(mkconfig) - run mkconfig with no parameters, capture all its output (which now subtly includes "stderr" in with the file output), and pass all-that as a string off to a utility that writes all that now-contaminated file as a raw file (instead of letting mkconfig do it via -o).

(I don't know that it's actually "stderr" or some progress data happening on stdout, so, quotes around "stderr" as "you get what I mean").

kenyon commented 1 year ago

@gcoxmoz thanks for the analysis.

It's definitely stderr, as you can see in the script it redirects output with >&2 for example here: https://salsa.debian.org/grub-team/grub/-/blob/9328b704e8eeb2d8e61f56aff0ad43dee3449632/util/grub-mkconfig.in#L262

glangloi commented 1 year ago

Hi, I've submitted #84 to fix this. Could someone review? Thanks!