> On Jun 7, 2017, at 1:23 PM, Daniel Kiper <daniel.ki...@oracle.com> wrote:
>
> On Mon, Jun 05, 2017 at 08:29:47AM -0600, Eric Snowberg wrote:
>>> On Jun 2, 2017, at 4:41 AM, Daniel Kiper <daniel.ki...@oracle.com> 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: /pci@306/pci@1/LSI,mrsas@0/disk@0: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//pci@306/pci@1
>>>> 001e3d0: 2f4c 5349 5c2c 6d72 7361 7340 302f 6469 /LSI\,mrsas@0/di
>>>> 001e3e0: 736b 4030 3a61 200a 7365 7420 7072 6566 sk@0: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//pci@306/pci@1/LSI,mrsas@0/disk@0:a
>>>>
>>>> The hint above is not correct. It should be:
>>>>
>>>> hint: ieee1275//pci@306/pci@1/LSI\,mrsas@0/disk@0: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//pci@306/pci@1/LSI.
>>>>
>>>> Signed-off-by: Eric Snowberg <eric.snowb...@oracle.com>
>>>> ---
>>>> 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?
>
> After your patch embedded config is parsed incorrectly, e.g. "set a=\f"
> sets $a to "\f" instead of "f”.
That is a useful config?
>
>>> 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.
>
> So, I think that you should store unescaped strings in core.img if it is
> required.
Throughout GRUB a comma is escaped unless it is before a partition number. If I
store it unescaped in the core.img I would still have the same problem above
when going into grub_disk_open.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
