Re: ELF bugfixes

[Robert Millan]

On Sat, Mar 21, 2009 at 06:58:58PM +0100, phcoder wrote:
> Robert Millan wrote:
>> On Wed, Mar 18, 2009 at 02:26:40PM +0100, phcoder wrote:
>>> Robert Millan wrote:
>>>> On Fri, Mar 13, 2009 at 09:52:39PM +0100, phcoder wrote:
>>>>> -  grub_multiboot_payload_entry_offset = ehdr->e_entry - 
>>>>> phdr(lowest_segment)->p_vaddr;
>>>>> +  for (i = 0; i < ehdr->e_phnum; i++)
>>>>> +    if (phdr(i)->p_vaddr <= ehdr->e_entry +      && phdr(i)->p_vaddr 
>>>>> + phdr(i)->p_memsz > ehdr->e_entry)
>>>>> +      grub_multiboot_payload_entry_offset = (ehdr->e_entry - 
>>>>> phdr(i)->p_vaddr)
>>>>> + + (phdr(i)->p_paddr  - phdr(lowest_segment)->p_paddr);
>>>> You need to handle the case in which grub_multiboot_payload_entry_offset 
>>>> is left
>>>> uninitialized (it needs to be initialized each time the multiboot command 
>>>> is
>>>> run, not just when the module is loaded).
>>>>
>>> module? actually it's when loading image. Perhaps you mean that   
>>> additional error check is necessary
>>
>> I meant GRUB's multiboot.mod, not the payload's module.  Sorry I wasn't 
>> clear.
>>
> With this error check if grub_multiboot_payload_entry_offset it can  
> happen only if no image is loaded. And actually  
> grub_multiboot_payload_entry_offset is set to 0 at multiboot.mod load
> So I don't really understand the problem

You can't rely on grub_multiboot_payload_entry_offset being set to 0, because
any subsequent call of "multiboot /something" has the potential to override
this.  You must not assume the multiboot command is only going to be run once.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."