grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] pass kernel command line as verbatim


From: Michael Chang
Subject: Re: [PATCH] pass kernel command line as verbatim
Date: Fri, 11 May 2018 15:01:03 +0800
User-agent: NeoMutt/20170421 (1.8.2)

On Wed, May 09, 2018 at 03:23:56PM +0200, Daniel Kiper wrote:
> On Thu, Apr 26, 2018 at 04:42:30PM +0800, Michael Chang wrote:
> > On Tue, Apr 24, 2018 at 12:03:21PM +0200, Daniel Kiper wrote:
> > > On Wed, Apr 18, 2018 at 03:07:15PM +0800, Michael Chang wrote:
> > > > On Tue, Apr 17, 2018 at 06:37:06PM +0200, Daniel Kiper wrote:
> > > > > On Wed, Apr 11, 2018 at 05:17:03PM +0800, Michael Chang wrote:
> > > > > > And this bug report seems relevant ..
> > > > > >
> > > > > > https://savannah.gnu.org/bugs/?49937
> > > > > >
> > > > > > On Wed, Apr 11, 2018 at 04:58:54PM +0800, Michael Chang wrote:
> > > > > > > The command line has been processed by grub shell, then the 
> > > > > > > result is expected
> > > > > > > to be passed to kernel command line as verbatim according to the 
> > > > > > > grub manual
> > > > > > > [1][2].
> > > > > > >
> > > > > > > This patch removes extra escape character added as it helps 
> > > > > > > nothing but only
> > > > > > > creates trouble as you want them to be literal. Besides the 
> > > > > > > surrounding
> > > > > > > double-quotes added is kept as it used to protect space.
> > > > >
> > > > > CC-ing Vladmir.
> > > > >
> > > > > Hmmm... Have you tested this patch on all platforms supported by 
> > > > > GRUB2?
> > > >
> > > > AFAICS, it affects boot protocols of which grub supports in commands 
> > > > like
> > > > linux, multiboot, xen_hypervisor and so on. Non of them will intepret 
> > > > escape
> > > > sequence and are pretty straightforward in composition -- In general, 
> > > > use space
> > > > to delimit options and quote the options by double quotes once they 
> > > > contain any
> > > > space.
> > > >
> > > > > I understand that current behavior is not accepted on some but I have
> > > > > a feeling that somebody make it work in that way due to some reason.
> > > > > Sadly I cannot find anything about that in git log. So, I have to 
> > > > > think
> > > > > how to cope with that. Or...
> > > >
> > > > As a side note, we had a down stream bug report that udev generates 
> > > > symlink
> > > > names containing '\' character, and with the current grub command 
> > > > processing
> > > > it's never possible to use those symlinks as kernel command line 
> > > > option, as the
> > > > character is either stripped or doubled.
> > >
> > > I understand your problem but I am afraid about compatibility. I would
> > > agree if you change the behavior here and introduce a shell variable to
> > > disable that change. If nobody will complain for some time then we can
> > > drop that variable a leave new desired behavior. Does it make sense?
> >
> > Yes, it makes sense. But here I have another idea that can be helpful if we
> > want to have one grub.cfg working on all versions. With the shell variable 
> > is
> > not possible as the result is either breaking in one way or not.
> >
> > It borrows the same idea of using feature variable to determine what current
> > build can support at run time, and thus allow to use another set of settings
> > like GRUB_CMDLINE_LINUX_V2 to override cmdline for the new sytax. It is 
> > fine to
> > not specify GRUB_CMDLINE_LINUX_V2, in that we will use GRUB_CMDLINE_LINUX.
> 
> I prefer something which is common for all boot protocols instead of
> something specific for a given one, in this case Linux one.

I think the same idea can extend to other protocol family's GRUB_CMDLINE_*
simple config settings, say GRUB_CMDLINE_XEN, GRUB_CMDLINE_NETBSD and so on,
here Linux is used as an example.

We probably need to consider whereas the grub.cfg could be sourced with
different versions of grub by which the flag is not intepreted.

> 
> > > And I have a question about space (what about tab?): why it is quoted
> > > if, AIUI, kernel command line should not be processed at this point?
> > > Should not it be created earlier properly? I mean with required quotes.
> >
> > The command line arguments will be processed by grub shell before passing 
> > them
> > to kernel command line. They will evaluate quotes (among other things) and
> > extract string from them. As a helper, the result can be observed by "echo"
> > command.
> >
> >   grub> echo abc="foo bar"
> >   abc=foo bar
> 
> Sure thing. So, I think that you have to escape all special characters
> for GRUB shell before putting a given command line into grub.cfg.
> I understand that it can be difficult in current shell based config
> generator but you should try...

Yes, we can accept it this way because we know what's happening. But for most
other people don't expect this to be true especially when the document is also
misleading. Also linux(bash) shell processing has added more complexity making
more confuse.

Simpl put, The abc="foo bar" (compared to abc=\"foo bar\") has been in
wildspread use as it's quite common in specifying command lines, changing it
here would likely introduce more impact for what has been deployed and in use
..

> 
> > As you can see, we have to add the quotes back here then it can be passed to
> > kernel command line as one option.
> >
> >   "abc=foo bar"
> 
> "abc=foo bar" != abc="foo bar". This behavior can potentially create
> issues on systems which use measurement/signing to protect the configs,
> etc. So, I think that command lines should be properly escaped before
> putting them into grub.cfg and GRUB should not be allowed to do anything
> with them including quoting.

Good catch. I think it is suffice to overwhelmed me on the attempt trying to
get less people impacted through keeping the behavior of automatic quoting
strings with space. 

> 
> > There could have other cases needs additional processing, but as a rule of
> > thumb I think we should avoid them as much as possible unless strickly
> > necessary. We need a "preditable" result that stays the same by means of 
> > echo
> > command's output.
> 
> Exactly!

Please note there's "as much as possible unless strictly necessary " in above
sentense but we may agree to have no exception here. :)

> 
> > For the question of tab, I did not investigate. I think it will be the same
> > with other characters unless it possessed special meaning to grub's shell
> > syntax...
> 
> Probably requires the same escaping/quoting as space. Of course before
> putting it into grub.cfg. Please look above for more details.

Yes.

> 
> > > Hmmm... What about modules command lines?
> >
> > I think it in common with others, Why is it special ?
> 
> I am asking because some boot protocols, e.g. Multiboot family, allows
> passing some arguments to modules too. So, these arguments should be
> treated in the same way as the kernel arguments are treated.

Yes, use Xen as an example, the module(xen kernel) shares the same
GRUB_CMDLINE_LINUX with linux kernel unless you're overriding them with
GRUB_CMDLINE_LINUX_XEN_REPLACE. There could have been other modules doing it
differently but I am not aware of ...

Thanks,
Michael

> 
> Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]