Re: [PATCH] access phdr header entries like an array

[Robert Millan]

Committed.

On Sun, Oct 14, 2007 at 03:48:18PM +0200, Robert Millan wrote:
> 
> This patch makes it easier and more intuitive to access entries in the phdr
> header as if it were a C array.  It has otherwise no effect on the current
> code (other than saving some space for an awkward reason), but is needed to
> implement the ability to load segments at an arbitrary address, distinguishing
> the relative offset rather than their absolute requested address.
> 
> I have the code for all the dance, including the ability to relocate the
> payload later on, but I've chosen to split it up for revision tracking
> purposes (besides, the rest needs quite a bit of cleanup yet ;-)).
> 
> I've checked there are no regressions (at least with invaders).  If there are
> no objections in a few days I'll check it in.
> 
> -- 
> Robert Millan
> 
> <GPLv2> I know my rights; I want my phone call!
> <DRM> What use is a phone call, if you are unable to speak?
> (as seen on /.)

> 2007-10-14  Robert Millan  <address@hidden>
> 
>       * loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): When loading
>       ELF segments, use a macro for arbitrarily accessing any of them instead
>       of preparing a pointer that allows access to one at a time.
>       (grub_multiboot_load_elf64): Likewise.
> 
> diff -ur grub2/loader/i386/pc/multiboot.c 
> grub2.phdr_array/loader/i386/pc/multiboot.c
> --- grub2/loader/i386/pc/multiboot.c  2007-07-25 21:29:24.000000000 +0200
> +++ grub2.phdr_array/loader/i386/pc/multiboot.c       2007-10-14 
> 15:32:37.000000000 +0200
> @@ -94,7 +94,7 @@
>  grub_multiboot_load_elf32 (grub_file_t file, void *buffer)
>  {
>    Elf32_Ehdr *ehdr = (Elf32_Ehdr *) buffer;
> -  Elf32_Phdr *phdr;
> +  void *phdr_base;
>    int i;
>  
>    if (ehdr->e_ident[EI_CLASS] != ELFCLASS32)
> @@ -112,35 +112,38 @@
>    
>    entry = ehdr->e_entry;
>    
> +  phdr_base = (void *) buffer + ehdr->e_phoff;
> +#define phdr(i)              ((Elf32_Phdr *) (phdr_base + (i) * 
> ehdr->e_phentsize))
> +
>    /* Load every loadable segment in memory.  */
>    for (i = 0; i < ehdr->e_phnum; i++)
>      {
> -      phdr = (Elf32_Phdr *) ((char *) buffer + ehdr->e_phoff
> -                          + i * ehdr->e_phentsize);
> -      if (phdr->p_type == PT_LOAD)
> +      if (phdr(i)->p_type == PT_LOAD)
>          {
>            /* The segment should fit in the area reserved for the OS.  */
> -          if ((phdr->p_paddr < grub_os_area_addr)
> -              || (phdr->p_paddr + phdr->p_memsz
> +          if ((phdr(i)->p_paddr < grub_os_area_addr)
> +              || (phdr(i)->p_paddr + phdr(i)->p_memsz
>                 > grub_os_area_addr + grub_os_area_size))
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "segment doesn't fit in memory reserved for the 
> OS");
>  
> -          if (grub_file_seek (file, (grub_off_t) phdr->p_offset)
> +          if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset)
>             == (grub_off_t) -1)
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "invalid offset in program header");
>         
> -          if (grub_file_read (file, (void *) phdr->p_paddr, phdr->p_filesz)
> -              != (grub_ssize_t) phdr->p_filesz)
> +          if (grub_file_read (file, (void *) phdr(i)->p_paddr, 
> phdr(i)->p_filesz)
> +              != (grub_ssize_t) phdr(i)->p_filesz)
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "couldn't read segment from file");
>  
> -          if (phdr->p_filesz < phdr->p_memsz)
> -            grub_memset ((char *) phdr->p_paddr + phdr->p_filesz, 0,
> -                      phdr->p_memsz - phdr->p_filesz);
> +          if (phdr(i)->p_filesz < phdr(i)->p_memsz)
> +            grub_memset ((char *) phdr(i)->p_paddr + phdr(i)->p_filesz, 0,
> +                      phdr(i)->p_memsz - phdr(i)->p_filesz);
>          }
>      }
> +
> +#undef phdr
>    
>    return grub_errno;
>  }
> @@ -158,7 +161,7 @@
>  grub_multiboot_load_elf64 (grub_file_t file, void *buffer)
>  {
>    Elf64_Ehdr *ehdr = (Elf64_Ehdr *) buffer;
> -  Elf64_Phdr *phdr;
> +  void *phdr_base;
>    int i;
>  
>    if (ehdr->e_ident[EI_CLASS] != ELFCLASS64)
> @@ -186,39 +189,42 @@
>  
>    entry = ehdr->e_entry;
>  
> +  phdr_base = (void *) buffer + ehdr->e_phoff;
> +#define phdr(i)              ((Elf64_Phdr *) (phdr_base + (i) * 
> ehdr->e_phentsize))
> +
>    /* Load every loadable segment in memory.  */
>    for (i = 0; i < ehdr->e_phnum; i++)
>      {
> -      phdr = (Elf64_Phdr *) ((char *) buffer + ehdr->e_phoff
> -                          + i * ehdr->e_phentsize);
> -      if (phdr->p_type == PT_LOAD)
> +      if (phdr(i)->p_type == PT_LOAD)
>          {
>            /* The segment should fit in the area reserved for the OS.  */
> -          if ((phdr->p_paddr < (grub_uint64_t) grub_os_area_addr)
> -              || (phdr->p_paddr + phdr->p_memsz
> +          if ((phdr(i)->p_paddr < (grub_uint64_t) grub_os_area_addr)
> +              || (phdr(i)->p_paddr + phdr(i)->p_memsz
>                 > ((grub_uint64_t) grub_os_area_addr
>                    + (grub_uint64_t) grub_os_area_size)))
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "segment doesn't fit in memory reserved for the 
> OS");
>         
> -       if (grub_file_seek (file, (grub_off_t) phdr->p_offset)
> +       if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset)
>             == (grub_off_t) -1)
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "invalid offset in program header");
>  
> -       if (grub_file_read (file, (void *) ((grub_uint32_t) phdr->p_paddr),
> -                           phdr->p_filesz)
> -              != (grub_ssize_t) phdr->p_filesz)
> +       if (grub_file_read (file, (void *) ((grub_uint32_t) phdr(i)->p_paddr),
> +                           phdr(i)->p_filesz)
> +              != (grub_ssize_t) phdr(i)->p_filesz)
>           return grub_error (GRUB_ERR_BAD_OS,
>                              "couldn't read segment from file");
>         
> -          if (phdr->p_filesz < phdr->p_memsz)
> -         grub_memset (((char *) ((grub_uint32_t) phdr->p_paddr)
> -                       + phdr->p_filesz),
> +          if (phdr(i)->p_filesz < phdr(i)->p_memsz)
> +         grub_memset (((char *) ((grub_uint32_t) phdr(i)->p_paddr)
> +                       + phdr(i)->p_filesz),
>                        0,
> -                      phdr->p_memsz - phdr->p_filesz);
> +                      phdr(i)->p_memsz - phdr(i)->p_filesz);
>          }
>      }
> +
> +#undef phdr
>    
>    return grub_errno;
>  }

> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
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."