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: Fri, 2 Jun 2017 12:41:21 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

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.
>
> 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
>
> The hint above is not correct.  It should be:
>
> hint: 
> 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.
>
> Signed-off-by: Eric Snowberg <address@hidden>
> ---
>  grub-core/kern/parser.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
> index 78175aa..be88baa 100644
> --- a/grub-core/kern/parser.c
> +++ b/grub-core/kern/parser.c
> @@ -30,7 +30,6 @@ static struct grub_parser_state_transition 
> state_transitions[] = {
>    {GRUB_PARSER_STATE_TEXT, GRUB_PARSER_STATE_QUOTE, '\'', 0},
>    {GRUB_PARSER_STATE_TEXT, GRUB_PARSER_STATE_DQUOTE, '\"', 0},
>    {GRUB_PARSER_STATE_TEXT, GRUB_PARSER_STATE_VAR, '$', 0},
> -  {GRUB_PARSER_STATE_TEXT, GRUB_PARSER_STATE_ESC, '\\', 0},

I have a feeling that this way you break parser for everybody. Could you
explain why it is valid for all? Or if issue is specific for SPARC only
please fix it in SPARC related code (e.g. store unescaped form in core.img).
I would prefer to not touch this file if we fix something only for one platform.

Daniel

PS Next time please CC on the patch Alex and Andrei too.



reply via email to

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