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

[Eric Snowberg]

> On Jun 2, 2017, at 4:41 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.
>> 
>> 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.

Can you explain what would break on other platforms with this change?

> Could you explain why it is valid for all?

Any platform containing a device with a comma in it would have the problem I 
identified above.