Which platform is it? i386-pc, i386-efi or x86_64-efi? The behavior is
actually will defined, just different between cpu modes
Le 3 nov. 2015 6:08 PM, "Andrei Borzenkov" <arvidj...@gmail.com> a écrit :

> 03.11.2015 19:28, Vladimir 'phcoder' Serbinenko пишет:
>
>> The code itself looks good but I'd like more details. Reading 0xffffffff
>> shouldn't cause reboot. Why does it?
>>
>
> That I do not know nor do I have access to system in question myself. I
> sent user patch that modified validate_header to do each comparison as
> individual statement and did line by line debug print (fortunately it was
> possible to connect serial port and capture output) and the last line
> printed was immediately before the very first
>
> head->magic == grub_cpu_to_be32_compile_time (CBFS_HEADER_MAGIC
>
> I suppose reading *one* byte from 0xffffffff should not cause issues but
> here we are reading 4 bytes which are beyond 0xffffffff. Who knows what
> memory controller in this system does in this case.
>
> Le 1 nov. 2015 3:53 PM, "Andrei Borzenkov" <arvidj...@gmail.com> a écrit :
>>
>> I was debugging problem reported by user on Dell Dimension 8300 - it
>>> rebooted when doing "ls -l". It turned out, the problem was triggered by
>>> loading cbfs which probed for header. System has 2GB memory, and attempt
>>> to
>>> read from address 0xffffffff caused instant reboot. 0xffffffff was
>>> returned
>>> by read from non-existing address 0xfffffffc.
>>>
>>> The proof of concept patch below avoids it, but I wonder what the proper
>>> fix is.
>>>
>>> diff --git a/grub-core/fs/cbfs.c b/grub-core/fs/cbfs.c
>>> index a34eb88..a5a2fde 100644
>>> --- a/grub-core/fs/cbfs.c
>>> +++ b/grub-core/fs/cbfs.c
>>> @@ -344,8 +344,9 @@ init_cbfsdisk (void)
>>>
>>>     ptr = *(grub_uint32_t *) 0xfffffffc;
>>>     head = (struct cbfs_header *) (grub_addr_t) ptr;
>>> +  grub_dprintf ("cbfs", "head=%p\n", head);
>>>
>>> -  if (!validate_head (head))
>>> +  if (0xffffffff - ptr < sizeof (*head) || !validate_head (head))
>>>       return;
>>>
>>>     cbfsdisk_size = ALIGN_UP (grub_be_to_cpu32 (head->romsize),
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to