grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ELF bugfixes


From: phcoder
Subject: Re: ELF bugfixes
Date: Thu, 12 Mar 2009 09:23:34 +0100
User-agent: Thunderbird 2.0.0.19 (X11/20090105)

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  <address@hidden>
+
+       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  <address@hidden>
 
        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
 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]