grub-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] parser: Remove escape from the state transitions


From: Daniel Kiper
Subject: Re: [PATCH] parser: Remove escape from the state transitions
Date: Thu, 12 Oct 2017 13:43:34 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Mon, Oct 09, 2017 at 09:23:44AM -0600, Eric Snowberg wrote:
> > On Oct 9, 2017, at 5:48 AM, Daniel Kiper <address@hidden> wrote:
> > On Fri, Oct 06, 2017 at 09:37:54AM -0600, Eric Snowberg wrote:
> >>> On Oct 6, 2017, at 8:47 AM, Daniel Kiper <address@hidden> wrote:
> >>> On Tue, May 30, 2017 at 04:07:52PM -0600, Eric Snowberg wrote:
> >>>> Remove GRUB_PARSER_STATE_ESC with state GRUB_PARSER_STATE_TEXT from
> >>>> the list of not allowed characters.
> >>>
> >>> Once again, NACK for this patch. I explained why earlier but...
> >>>
> >>>> This fixes a problem where a properly escaped comma is in the disk path.
> >>>>
> >>>> For example: 
> >>>> /address@hidden/address@hidden/LSI,address@hidden/address@hidden:a
> >>>>
> >>>> During grub install, the search.fs_uuid is correctly stored in the
> >>>> core.img.
> >>>>
> >>>> As seen here:
> >>>>
> >>>> 001e380: 7365 6172 6368 2e66 735f 7575 6964 2039  search.fs_uuid 9
> >>>> 001e390: 6462 6137 6333 362d 6431 6432 2d34 6163  dba7c36-d1d2-4ac
> >>>> 001e3a0: 642d 6135 3037 2d30 3634 6132 3462 3538  d-a507-064a24b58
> >>>> 001e3b0: 3666 3420 726f 6f74 2069 6565 6531 3237  6f4 root ieee127
> >>>> 001e3c0: 352f 2f70 6369 4033 3036 2f70 6369 4031  
> >>>> 5//address@hidden/address@hidden
> >>>> 001e3d0: 2f4c 5349 5c2c 6d72 7361 7340 302f 6469  /LSI\,address@hidden/di
> >>>> 001e3e0: 736b 4030 3a61 200a 7365 7420 7072 6566  address@hidden:a .set 
> >>>> pref
> >>>> 001e3f0: 6978 3d28 2472 6f6f 7429 272f 6772 7562  ix=($root)'/grub
> >>>> 001e400: 3227 0a00 0000 0000 0000 0003 0000 0010  2'..............
> >>>> 001e410: 2f67 7275 6232 0000                      /grub2..
> >>>>
> >>>> Before this change the following args would be sent to
> >>>> grub_cmd_do_search:
> >>>>
> >>>> key: 9dba7c36-d1d2-4acd-a507-064a24b586f4
> >>>> var: root
> >>>> hint: 
> >>>> ieee1275//address@hidden/address@hidden/LSI,address@hidden/address@hidden:a
> >>>
> >>> ...because hint should be quoted in core.img using double quotes or even 
> >>> single quotes...
> >>> Or every control char should be escaped. Normal shell rules apply here.
> >>
> >> Hints are written during the install into the core.img. Once the system
> >> boots, the parser is used to retrieve information from the core.img.
> >> Currently the parser will strip double quotes, single quotes and escapes.
> >> So I don’t understand how you recommend fixing this then.
> >
> > Could you send me or point a script which creates embedded config for you?
>
> There is no script.
>
> As I explained in the patch.  If your boot device name has a comma, which
> it does with a Megaraid, you can not boot GRUB.
>
> Install as follows:
>
> $ grub-install —force /dev/sda1
>
> By default it creates a core.img with what I provided in the git comment:
>
> 001e380: 7365 6172 6368 2e66 735f 7575 6964 2039  search.fs_uuid 9
> 001e390: 6462 6137 6333 362d 6431 6432 2d34 6163  dba7c36-d1d2-4ac
> 001e3a0: 642d 6135 3037 2d30 3634 6132 3462 3538  d-a507-064a24b58
> 001e3b0: 3666 3420 726f 6f74 2069 6565 6531 3237  6f4 root ieee127
> 001e3c0: 352f 2f70 6369 4033 3036 2f70 6369 4031  
> 5//address@hidden/address@hidden
> 001e3d0: 2f4c 5349 5c2c 6d72 7361 7340 302f 6469  /LSI\,address@hidden/di
> 001e3e0: 736b 4030 3a61 200a 7365 7420 7072 6566  address@hidden:a .set pref
> 001e3f0: 6978 3d28 2472 6f6f 7429 272f 6772 7562  ix=($root)'/grub
> 001e400: 3227 0a00 0000 0000 0000 0003 0000 0010  2'..............
> 001e410: 2f67 7275 6232 0000                      /grub2..
>
> As you can see, everything is escaped as GRUB expects.
>
> Now during boot, the parser is used.  Without my patch, it will strip the \,.

AIUI you mean it strips '\'. If yes then it is correct behavior. And it
should stay as is. If you wish to leave '\' you have to quote hint.
Hence probably you have to fiddle with grub-install code or whatever
creates the hint. Or the hint consumer code to properly consume ',' alone.

> So it changes the hint from:
>
> ieee1275//address@hidden/address@hidden/LSI,address@hidden/address@hidden:a
> to
> ieee1275//address@hidden/address@hidden/LSI\,address@hidden/address@hidden:a
>
> Later on, when it tries to use this disk, it incorrectly truncates
> the device name since the comma isn’t escaped and tries to do the
> grub_disk_open with ieee1275//address@hidden/address@hidden/LSI.

I am not sure who strips everything after the ','. Whoever it is it is
not the parser for sure. There is a chance that you should look for
problem here.

Daniel



reply via email to

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