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: Daniel Kiper
Subject: Re: [PATCH] pass kernel command line as verbatim
Date: Wed, 9 May 2018 15:23:56 +0200
User-agent: Mutt/1.3.28i

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.

> > 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...

> 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.

> 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!

> 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.

> > 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.

Daniel



reply via email to

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