Bug 923374 - grub2 improperly escapes spaces in kernel parameters
Summary: grub2 improperly escapes spaces in kernel parameters
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: grub2
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Peter Jones
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1061925 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-19 16:58 UTC by Danny Baumann
Modified: 2014-07-28 11:09 UTC (History)
11 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-06-30 20:06:20 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
script to create sysilnux-extlinux config file (1.92 KB, application/x-shellscript)
2013-12-19 18:43 UTC, Ken
no flags Details
Updated version of 0066-Pass-x-hex-hex-straight-through-unmolested.patch fixing this bug (4.39 KB, patch)
2014-06-23 09:13 UTC, Hans de Goede
no flags Details | Diff

Description Danny Baumann 2013-03-19 16:58:04 UTC
Description of problem:
In order to work around an ACPI problem, I'm trying to boot my kernel with the parameter "acpi_osi=!Windows 2012" to turn off Windows 8 announcement. No matter what form of escaping I tried, grub2 always replaces the space by \x20.

Version-Release number of selected component (if applicable):
grub2-efi-2.00-15.fc18.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Try to set the kernel parameter acpi_osi="!Windows 2012", either by entering it in grub's command line editor or by placing it in /etc/default/grub
2. Boot the kernel
  
Actual results:
- acpi_osi="!Windows 2012" is converted to "acpi_osi=!Windows\x202012"
- "acpi_osi=!Windows 2012" is converted to "acpi_osi=!Windows\x202012"
- acpi_osi=!Windows\ 2012 is converted to "acpi_osi=!Windows\x202012"
- acpi_osi="!Windows\ 2012" is converted to "acpi_osi=!Windows\\\x202012"

I've verified (by adding a printk statement) that the \x20 is indeed reaching the code evaluating the parameter, effectively breaking it.

Expected results:
There should be a way of passing "acpi_osi=!Windows 2012" to the kernel.

Comment 1 Peter Jones 2013-04-03 21:27:48 UTC
The problem here is that grub's idea of what needs to be escaped and how and the kernel's don't always match.

My first thought is that it'll be difficult to do anything in grub2 to avoid this - the quoting code is really quite difficult, and most changes to fix this will break 855849 .  One answer would be to handle that line with its own parser that handles the kernel's behaviour, but a lot of grub2 would require re-working to make it not be un-escaped before it gets there.

Another possibility, which may be significantly simpler, would be to make the kernel handle \x20 as a space when parsing acpi_osi.

Comment 2 Danny Baumann 2013-05-06 15:35:16 UTC
Is there an upstream report about this? I couldn't find any unescaping code for \xYY in the kernel (please tell me if I missed it :) ), so this kind of escaping never is something that's suitable, it seems?

Comment 3 Cesar Eduardo Barros 2013-07-21 01:12:27 UTC
Also broken in Fedora 19. It worked on Fedora 17 with acpi_osi="!Windows 2009" (on grub.cfg) and acpi_osi=\"!Windows 2009\" (on /etc/default/grub).

Comment 4 Ken 2013-12-18 19:15:38 UTC
Broken in Fedora 20.

I tried to do dyndbg="module xhci_hcd +p" , but got dyndbg="module\x20xhci_hcd\x20+p"

Gummiboot works fine.

Comment 5 Ken 2013-12-19 18:43:45 UTC
Created attachment 839168 [details]
script to create sysilnux-extlinux config file

Comment 6 Ken 2013-12-19 18:45:46 UTC
syslinux-extlinux works fine as well.

FYI, to set up extlinux :

yum install syslinux syslinux-extlinux
extlinux --install /boot/extlinux   ## creates /boot/extlinux/ldlinux.sys

disk0=/dev/sda
dd bs=440 conv=notrunc count=1 if=/usr/share/syslinux/gptmbr_c.bin of=$disk0

num0=3 ## partition number of /boot
sgdisk $disk0 --attributes=$num0:set:2    # On GPT disks, set "Legacy boot"


create config file : /boot/extlinux/extlinux.conf .  I've attached a simple script to create the config file

Comment 7 Fedora End Of Life 2013-12-21 12:18:06 UTC
This message is a reminder that Fedora 18 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 18. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '18'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 18's end of life.

Thank you for reporting this issue and we are sorry that we may not be 
able to fix it before Fedora 18 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior to Fedora 18's end of life.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 8 Fedora End Of Life 2014-02-05 20:07:25 UTC
Fedora 18 changed to end-of-life (EOL) status on 2014-01-14. Fedora 18 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 9 Cesar Eduardo Barros 2014-02-05 21:37:22 UTC
Filed bug 1061925 against Fedora 19.

Comment 10 Hans de Goede 2014-06-23 08:14:37 UTC
(In reply to Cesar Eduardo Barros from comment #9)
> Filed bug 1061925 against Fedora 19.

It is better to just re-open this bug, doing so now.

Comment 11 Hans de Goede 2014-06-23 08:15:08 UTC
*** Bug 1061925 has been marked as a duplicate of this bug. ***

Comment 12 Hans de Goede 2014-06-23 08:31:12 UTC
Peter,

This is really a bad bug, and we really ought to fix this. While trying to debug bug 1110011 with the user reporting it I got stuck since it is impossible to use the dyndb kernel commandline option due to this bug.

Likewise using the acpi_osi option is not possible because of this bug.

Having the kernel parse \x20 is not the answer here, that just makes no sense at all the \x20 in the labels with spaces comes from the udev generator for /dev/disk/by-label expecting all kernel commandline parsing code which may encounter spaces to deal with this just is non sense, esp. since it is not a kernel convention to have the \x20 in the first place.

I've been looking at into the cause of this, and this is caused by a Fedora specific patch, to be precise this is caused (looking at fedpkg master) by 0066-Pass-x-hex-hex-straight-through-unmolested.patch.

This patch does what its Subject of: "Pass "\x[[:hex:]][[:hex:]]" straight through unmolested." says it does in number of places. But for some reason in one place it goes beyond that and also replaces a space with \x20 without any explanation why.

I can understand that your worried about causing regressions wrt bug 855849, but anaconda was already putting \x20 in the grub.cfg there, the problem was the \ being eaten, there is no reason to do any escaping of spaces in grub itself to fix bug 855849, only to keep existing \x## sequences as is.

Moreover the code path which also does the space mangling is only used in grub-efi, not in regular grub, so if replacing spaces with \x20 is something which grub must do for things to work, then we should already have a large amount of bugs from classic boot users.

Summarizing: I believe that this is an important bug to fix, and the fix should be as simple as removing the snipped of code in 0066-Pass-x-hex-hex-straight-through-unmolested.patch which expands spaces to \x20, while leaving all the code (which is done in much more places) to preserve existing \x## sequences in there.

I've started a scratchbuild with a modified 0066...patch, I'll report back here if that fixes things and also provide a link for people who want to give that build a shot.

Thanks & Regards,

Hans

Comment 13 Hans de Goede 2014-06-23 09:13:00 UTC
Created attachment 911363 [details]
Updated version of 0066-Pass-x-hex-hex-straight-through-unmolested.patch fixing this bug

Comment 14 Hans de Goede 2014-06-23 09:15:17 UTC
Peter,

I can now confirm that the updated version of 0066-Pass-x-hex-hex-straight-through-unmolested.patch which I've just attached fixes this. Can you please issue a grub update with this fix ? Or if your short on time, can you review the change (dropping of the space expansion snippet), and let me know if it is ok if I create an update with this updated patch ?

Thanks,

Hans

Comment 15 Hans de Goede 2014-06-23 09:19:41 UTC
For people following this bug who are interested in testing a fixed version of grub, I've put a copy of the scratch build I've done here: http://people.fedoraproject.org/~jwrdegoede/grub2-rhbz923374/

To test this do the following:

1) Run "rpm -qa | grep grub2" to see which grub2 packages you've installed
2) Download the relevant packages
3) run "rpm -Uvh grub2*.rpm" to install the new version
4) reboot

You now should be able to use kernel cmdline arguments with spaces in there without issues.

Note:
1) Upgrading your bootloader always brings a risk of ending up with a non bootable machine with it !
2) This is a build for f21, but you can install it on f20 without problems
3) No need to run grub2-install since this only affects machines booting through EFI, where grub2-install is not necessary

Comment 16 Hans de Goede 2014-06-30 20:06:20 UTC
Peter has just done a build of grub2 with my fix for this in, closing this.

Note there are no plans to do an update for F-20 for this, if you really need the fix simply download & install the F-21 packages on your F-20 system.

Comment 17 Alexander Lindqvist 2014-07-27 15:44:04 UTC
(In reply to Hans de Goede from comment #16)
> Peter has just done a build of grub2 with my fix for this in, closing this.
> 
> Note there are no plans to do an update for F-20 for this, if you really
> need the fix simply download & install the F-21 packages on your F-20 system.

Hello

I first tried grub2-2.02-0.6 from Fedora 21 and then your patched version grub2-2.02-0.3.fc21.hdg923374.x86_64.rpm. But both still expands spaces. Do I need to do something more?

This is on Fedora 20.

Comment 18 Hans de Goede 2014-07-27 16:01:59 UTC
(In reply to Alexander Lindqvist from comment #17)
> (In reply to Hans de Goede from comment #16)
> > Peter has just done a build of grub2 with my fix for this in, closing this.
> > 
> > Note there are no plans to do an update for F-20 for this, if you really
> > need the fix simply download & install the F-21 packages on your F-20 system.
> 
> Hello
> 
> I first tried grub2-2.02-0.6 from Fedora 21 and then your patched version
> grub2-2.02-0.3.fc21.hdg923374.x86_64.rpm. But both still expands spaces. Do
> I need to do something more?

Are you using EFI to boot your system ? AFAIK the expand spaces bug was only ever present in EFI builds, but maybe for older grub versions non EFI boot had the problem too ?

If you're not using EFI boot you need to run grub-install to update grub from the updated binaries, see:
https://fedoraproject.org/wiki/GRUB_2

If you're using EFI, things should just work after updating grub, unless for some reason you /boot/EFI is not being mounted properly.

Comment 19 Cesar Eduardo Barros 2014-07-27 21:54:58 UTC
(In reply to Hans de Goede from comment #18)
> Are you using EFI to boot your system ? AFAIK the expand spaces bug was only
> ever present in EFI builds, but maybe for older grub versions non EFI boot
> had the problem too ?

See bug 1061925, which was marked as a duplicate of this one. It's on a laptop which does not even have EFI at all. So, this bug did exist on non-EFI, at least on F18/F19.

Comment 20 Hans de Goede 2014-07-28 08:28:31 UTC
Hi,

(In reply to Cesar Eduardo Barros from comment #19)
> See bug 1061925, which was marked as a duplicate of this one. It's on a
> laptop which does not even have EFI at all. So, this bug did exist on
> non-EFI, at least on F18/F19.

Ah then it got fixed on non-EFI, but not on EFI before I got involved in this bug. So then the fix indeed is to install the new F-21 grub, and then run grub-install as described here:
https://fedoraproject.org/wiki/GRUB_2

Note this is not 100% without risk, but normally it should work fine.

Regards,

Hans

Comment 21 Alexander Lindqvist 2014-07-28 11:09:44 UTC
I did A grub-install yesterday and that solved the problem just before I saw your comment. I was not using EFI boot and the bug was present on this FC20 system.

Works after update.


Note You need to log in before you can comment on or make changes to this bug.