Fixed
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
- 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
--
Regards
Vladimir 'phcoder' Serbinenko
Index: include/grub/elf.h
===================================================================
--- include/grub/elf.h (revision 2036)
+++ include/grub/elf.h (working copy)
@@ -77,7 +77,7 @@
Elf32_Half e_shentsize; /* Section header table entry size */
Elf32_Half e_shnum; /* Section header table entry count */
Elf32_Half e_shstrndx; /* Section header string table index */
-} Elf32_Ehdr;
+} __attribute__ ((packed)) Elf32_Ehdr;
typedef struct
{
@@ -95,7 +95,7 @@
Elf64_Half e_shentsize; /* Section header table entry size */
Elf64_Half e_shnum; /* Section header table entry count */
Elf64_Half e_shstrndx; /* Section header string table index */
-} Elf64_Ehdr;
+} __attribute__ ((packed)) Elf64_Ehdr;
/* Fields in the e_ident array. The EI_* macros are indices into the
array. The macros under each EI_* macro are the values the byte
@@ -272,7 +272,7 @@
Elf32_Word sh_info; /* Additional section information */
Elf32_Word sh_addralign; /* Section alignment */
Elf32_Word sh_entsize; /* Entry size if section holds table */
-} Elf32_Shdr;
+} __attribute__ ((packed)) Elf32_Shdr;
typedef struct
{
@@ -286,7 +286,7 @@
Elf64_Word sh_info; /* Additional section information */
Elf64_Xword sh_addralign; /* Section alignment */
Elf64_Xword sh_entsize; /* Entry size if section holds table */
-} Elf64_Shdr;
+} __attribute__ ((packed)) Elf64_Shdr;
/* Special section indices. */
@@ -367,7 +367,7 @@
unsigned char st_info; /* Symbol type and binding */
unsigned char st_other; /* Symbol visibility */
Elf32_Section st_shndx; /* Section index */
-} Elf32_Sym;
+} __attribute__ ((packed)) Elf32_Sym;
typedef struct
{
@@ -377,7 +377,7 @@
Elf64_Section st_shndx; /* Section index */
Elf64_Addr st_value; /* Symbol value */
Elf64_Xword st_size; /* Symbol size */
-} Elf64_Sym;
+} __attribute__ ((packed)) Elf64_Sym;
/* The syminfo section if available contains additional information about
every dynamic symbol. */
@@ -386,13 +386,13 @@
{
Elf32_Half si_boundto; /* Direct bindings, symbol bound to */
Elf32_Half si_flags; /* Per symbol flags */
-} Elf32_Syminfo;
+} __attribute__ ((packed)) Elf32_Syminfo;
typedef struct
{
Elf64_Half si_boundto; /* Direct bindings, symbol bound to */
Elf64_Half si_flags; /* Per symbol flags */
-} Elf64_Syminfo;
+} __attribute__ ((packed)) Elf64_Syminfo;
/* Possible values for si_boundto. */
#define SYMINFO_BT_SELF 0xffff /* Symbol bound to self */
@@ -477,7 +477,7 @@
{
Elf32_Addr r_offset; /* Address */
Elf32_Word r_info; /* Relocation type and symbol index */
-} Elf32_Rel;
+} __attribute__ ((packed)) Elf32_Rel;
/* I have seen two different definitions of the Elf64_Rel and
Elf64_Rela structures, so we'll leave them out until Novell (or
@@ -488,7 +488,7 @@
{
Elf64_Addr r_offset; /* Address */
Elf64_Xword r_info; /* Relocation type and symbol index */
-} Elf64_Rel;
+} __attribute__ ((packed)) Elf64_Rel;
/* Relocation table entry with addend (in section of type SHT_RELA). */
@@ -497,14 +497,14 @@
Elf32_Addr r_offset; /* Address */
Elf32_Word r_info; /* Relocation type and symbol index */
Elf32_Sword r_addend; /* Addend */
-} Elf32_Rela;
+} __attribute__ ((packed)) Elf32_Rela;
typedef struct
{
Elf64_Addr r_offset; /* Address */
Elf64_Xword r_info; /* Relocation type and symbol index */
Elf64_Sxword r_addend; /* Addend */
-} Elf64_Rela;
+} __attribute__ ((packed)) Elf64_Rela;
/* How to extract and insert information held in the r_info field. */
@@ -528,7 +528,7 @@
Elf32_Word p_memsz; /* Segment size in memory */
Elf32_Word p_flags; /* Segment flags */
Elf32_Word p_align; /* Segment alignment */
-} Elf32_Phdr;
+} __attribute__ ((packed)) Elf32_Phdr;
typedef struct
{
@@ -540,7 +540,7 @@
Elf64_Xword p_filesz; /* Segment size in file */
Elf64_Xword p_memsz; /* Segment size in memory */
Elf64_Xword p_align; /* Segment alignment */
-} Elf64_Phdr;
+} __attribute__ ((packed)) Elf64_Phdr;
/* Legal values for p_type (segment type). */
@@ -604,7 +604,7 @@
Elf32_Word d_val; /* Integer value */
Elf32_Addr d_ptr; /* Address value */
} d_un;
-} Elf32_Dyn;
+} __attribute__ ((packed)) Elf32_Dyn;
typedef struct
{
@@ -614,7 +614,7 @@
Elf64_Xword d_val; /* Integer value */
Elf64_Addr d_ptr; /* Address value */
} d_un;
-} Elf64_Dyn;
+} __attribute__ ((packed)) Elf64_Dyn;
/* Legal values for d_tag (dynamic entry type). */
@@ -770,7 +770,7 @@
Elf32_Word vd_aux; /* Offset in bytes to verdaux array */
Elf32_Word vd_next; /* Offset in bytes to next verdef
entry */
-} Elf32_Verdef;
+} __attribute__ ((packed)) Elf32_Verdef;
typedef struct
{
@@ -782,7 +782,7 @@
Elf64_Word vd_aux; /* Offset in bytes to verdaux array */
Elf64_Word vd_next; /* Offset in bytes to next verdef
entry */
-} Elf64_Verdef;
+} __attribute__ ((packed)) Elf64_Verdef;
/* Legal values for vd_version (version revision). */
@@ -807,14 +807,14 @@
Elf32_Word vda_name; /* Version or dependency names */
Elf32_Word vda_next; /* Offset in bytes to next verdaux
entry */
-} Elf32_Verdaux;
+} __attribute__ ((packed)) Elf32_Verdaux;
typedef struct
{
Elf64_Word vda_name; /* Version or dependency names */
Elf64_Word vda_next; /* Offset in bytes to next verdaux
entry */
-} Elf64_Verdaux;
+} __attribute__ ((packed)) Elf64_Verdaux;
/* Version dependency section. */
@@ -828,7 +828,7 @@
Elf32_Word vn_aux; /* Offset in bytes to vernaux array */
Elf32_Word vn_next; /* Offset in bytes to next verneed
entry */
-} Elf32_Verneed;
+} __attribute__ ((packed)) Elf32_Verneed;
typedef struct
{
@@ -839,7 +839,7 @@
Elf64_Word vn_aux; /* Offset in bytes to vernaux array */
Elf64_Word vn_next; /* Offset in bytes to next verneed
entry */
-} Elf64_Verneed;
+} __attribute__ ((packed)) Elf64_Verneed;
/* Legal values for vn_version (version revision). */
@@ -857,7 +857,7 @@
Elf32_Word vna_name; /* Dependency name string offset */
Elf32_Word vna_next; /* Offset in bytes to next vernaux
entry */
-} Elf32_Vernaux;
+} __attribute__ ((packed)) Elf32_Vernaux;
typedef struct
{
@@ -867,7 +867,7 @@
Elf64_Word vna_name; /* Dependency name string offset */
Elf64_Word vna_next; /* Offset in bytes to next vernaux
entry */
-} Elf64_Vernaux;
+} __attribute__ ((packed)) Elf64_Vernaux;
/* Legal values for vna_flags. */
@@ -892,7 +892,7 @@
void *a_ptr; /* Pointer value */
void (*a_fcn) (void); /* Function pointer value */
} a_un;
-} Elf32_auxv_t;
+} __attribute__ ((packed)) Elf32_auxv_t;
typedef struct
{
@@ -903,7 +903,7 @@
void *a_ptr; /* Pointer value */
void (*a_fcn) (void); /* Function pointer value */
} a_un;
-} Elf64_auxv_t;
+} __attribute__ ((packed)) Elf64_auxv_t;
/* Legal values for a_type (entry type). */
@@ -951,14 +951,14 @@
Elf32_Word n_namesz; /* Length of the note's name. */
Elf32_Word n_descsz; /* Length of the note's descriptor. */
Elf32_Word n_type; /* Type of the note. */
-} Elf32_Nhdr;
+} __attribute__ ((packed)) Elf32_Nhdr;
typedef struct
{
Elf64_Word n_namesz; /* Length of the note's name. */
Elf64_Word n_descsz; /* Length of the note's descriptor. */
Elf64_Word n_type; /* Type of the note. */
-} Elf64_Nhdr;
+} __attribute__ ((packed)) Elf64_Nhdr;
/* Known names of notes. */
@@ -1000,7 +1000,7 @@
Elf32_Word m_poffset; /* Symbol offset. */
Elf32_Half m_repeat; /* Repeat count. */
Elf32_Half m_stride; /* Stride info. */
-} Elf32_Move;
+} __attribute__ ((packed)) Elf32_Move;
typedef struct
{
@@ -1009,7 +1009,7 @@
Elf64_Xword m_poffset; /* Symbol offset. */
Elf64_Half m_repeat; /* Repeat count. */
Elf64_Half m_stride; /* Stride info. */
-} Elf64_Move;
+} __attribute__ ((packed)) Elf64_Move;
/* Macro to construct move records. */
#define ELF32_M_SYM(info) ((info) >> 8)
@@ -1369,7 +1369,7 @@
Elf32_Word gt_g_value; /* If this value were used for -G */
Elf32_Word gt_bytes; /* This many bytes would be used */
} gt_entry; /* Subsequent entries in section */
-} Elf32_gptab;
+} __attribute__ ((packed)) Elf32_gptab;
/* Entry found in sections of type SHT_MIPS_REGINFO. */
@@ -1378,7 +1378,7 @@
Elf32_Word ri_gprmask; /* General registers used */
Elf32_Word ri_cprmask[4]; /* Coprocessor registers used */
Elf32_Sword ri_gp_value; /* $gp register value */
-} Elf32_RegInfo;
+} __attribute__ ((packed)) Elf32_RegInfo;
/* Entries found in sections of type SHT_MIPS_OPTIONS. */
@@ -1390,7 +1390,7 @@
Elf32_Section section; /* Section header index of section affected,
0 for global options. */
Elf32_Word info; /* Kind-specific information. */
-} Elf_Options;
+} __attribute__ ((packed)) Elf_Options;
/* Values for `kind' field in Elf_Options. */
@@ -1437,7 +1437,7 @@
{
Elf32_Word hwp_flags1; /* Extra flags. */
Elf32_Word hwp_flags2; /* Extra flags. */
-} Elf_Options_Hw;
+} __attribute__ ((packed)) Elf_Options_Hw;
/* Masks for `info' in ElfOptions for ODK_HWAND and ODK_HWOR entries. */
@@ -1579,7 +1579,7 @@
Elf32_Word l_checksum; /* Checksum */
Elf32_Word l_version; /* Interface version */
Elf32_Word l_flags; /* Flags */
-} Elf32_Lib;
+} __attribute__ ((packed)) Elf32_Lib;
typedef struct
{
@@ -1588,7 +1588,7 @@
Elf64_Word l_checksum; /* Checksum */
Elf64_Word l_version; /* Interface version */
Elf64_Word l_flags; /* Flags */
-} Elf64_Lib;
+} __attribute__ ((packed)) Elf64_Lib;
/* Legal values for l_flags. */
Index: ChangeLog
===================================================================
--- ChangeLog (revision 2036)
+++ ChangeLog (working copy)
@@ -1,3 +1,12 @@
+2009-03-01 Vladimir Serbinenko <phco...@gmail.com>
+
+ Bugfixes in multiboot for bugs uncovered by solaris kernel
+
+ * loader/i386/multiboot_elfxx.c (grub_multiboot_load_elf): corrected
+ limit detection
+ Use vaddr of correct segment for entry_point
+ * include/grub/elf.h: added missing __attribute__ ((packed))
+
2009-03-12 Vladimir Serbinenko <phco...@gmail.com>
Parttool
Index: loader/i386/multiboot_elfxx.c
===================================================================
--- loader/i386/multiboot_elfxx.c (revision 2036)
+++ loader/i386/multiboot_elfxx.c (working copy)
@@ -49,7 +49,7 @@
{
Elf_Ehdr *ehdr = (Elf_Ehdr *) buffer;
char *phdr_base;
- int lowest_segment = 0, highest_segment = 0;
+ int lowest_segment = -1, highest_segment = -1;
int i;
if (ehdr->e_ident[EI_CLASS] != ELFCLASSXX)
@@ -83,9 +83,11 @@
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;
}
code_size = (phdr(highest_segment)->p_paddr + phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr;
@@ -105,8 +107,8 @@
{
char *load_this_module_at = (char *) (grub_multiboot_payload_orig + (long) (phdr(i)->p_paddr - phdr(lowest_segment)->p_paddr));
- grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, memsz=0x%lx\n",
- i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz);
+ grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, memsz=0x%lx, vaddr=0x%lx\n",
+ i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, (long) phdr(i)->p_vaddr);
if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset)
== (grub_off_t) -1)
@@ -124,7 +126,11 @@
}
}
- 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);
#undef phdr
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
