On Wed, Mar 11, 2009 at 10:21:41PM +0100, phcoder wrote:
> Robert Millan wrote:
>> On Mon, Mar 02, 2009 at 01:35:06AM +0100, phcoder wrote:
>>> + * include/grub/elf.h: added missing attributes
>>
>> This should be a bit more descriptive.
>>
>>> for (i = 0; i < ehdr->e_phnum; i++)
>>> if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0)
>>> {
>>> - if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr)
>>> + if (lowest_segment == -1 + || phdr(i)->p_paddr <
>>> phdr(lowest_segment)->p_paddr)
>>> lowest_segment = i;
>>> - if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
>>> + if (highest_segment == -1
>>> + || phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
>>> highest_segment = i;
>>> }
>>
>> Why?
>
> Because if first segment doesn't have the PT_LOAD attribute set then it
> should be considered in this comparison
But you didn't remove the PT_LOAD check. And in the routine below that
does the actual segment load, we still check for PT_LOAD. Those should be
consistent, right?
>>> - grub_multiboot_payload_entry_offset = ehdr->e_entry -
>>> phdr(lowest_segment)->p_vaddr;
>>> + grub_multiboot_payload_entry_offset = ehdr->e_entry -
>>> phdr(lowest_segment)->p_paddr;
>>
>> Are you sure about this? IIRC e_entry is in the virtual address space. I
>> think we had some trouble with this (with NetBSD?), which lead to the current
>> use of p_vaddr in this line.
>>
> Actually now thinking I see that the problem is more deep. The section
> which is loaded at the lowest address isn't necessarily the section
> which contains entry point. I'll fix this part cleanly and will resubmit
> the patch
No, but AFAICT the entry point is defined relative to that address, regardless
of which segment contains it.
--
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."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
