Re: [PATCH v2] Suport for bi-endianess in elf file

[Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com>]

On 2015-07-24 16:00, Vladimir 'phcoder' Serbinenko wrote:
>
> Le 23 juil. 2015 16:11, "Paulo Flabiano Smorigo"
> <pfsmorigo@linux.vnet.ibm.com <mailto:pfsmorigo@linux.vnet.ibm.com>> a
> écrit :
>  >
>  > Updated version with the suggestions from Andrei Borzenkov.
>  >
>  > * grub-core/kern/elf.c: check and switch endianess with grub_{be,le}_to
>  > cpu functions.
>  > * grub-core/kern/elfXX.c: Likewise.
>  >
>  > Also-by: Tomohiro B Berry <tbberry@us.ibm.com
> <mailto:tbberry@us.ibm.com>>
>  > ---
>  >  grub-core/kern/elf.c   | 51 ++++++++++++++++++++++++++++++---
>  >  grub-core/kern/elfXX.c | 76
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  >  2 files changed, 123 insertions(+), 4 deletions(-)
>  >
>  > diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
>  > index 5f99c43..1be9c1c 100644
>  > --- a/grub-core/kern/elf.c
>  > +++ b/grub-core/kern/elf.c
>  > @@ -28,6 +28,11 @@
>  >
>  >  GRUB_MOD_LICENSE ("GPLv3+");
>  >
>  > +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
>  > +void grub_elf32_check_endianess (grub_elf_t elf);
>  > +void grub_elf64_check_endianess (grub_elf_t elf);
>  > +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
>  > +
>  >  /* Check if EHDR is a valid ELF header.  */
>  >  static grub_err_t
>  >  grub_elf_check_header (grub_elf_t elf)
>  > @@ -38,8 +43,19 @@ grub_elf_check_header (grub_elf_t elf)
>  >        || e->e_ident[EI_MAG1] != ELFMAG1
>  >        || e->e_ident[EI_MAG2] != ELFMAG2
>  >        || e->e_ident[EI_MAG3] != ELFMAG3
>  > -      || e->e_ident[EI_VERSION] != EV_CURRENT
>  > -      || e->e_version != EV_CURRENT)
>  > +      || e->e_ident[EI_VERSION] != EV_CURRENT)
>  > +    return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-independent
> ELF magic"));
>  > +
>  > +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
>  > +  if (grub_elf_is_elf32 (elf))
>  > +      grub_elf32_check_endianess (elf);
>  > +  else if (grub_elf_is_elf64 (elf))
>  > +      grub_elf64_check_endianess (elf);
>  > +  else
>  > +    return grub_error (GRUB_ERR_BAD_OS, N_("Uknown ELF class"));
>  > +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
>  > +
>  > +  if (e->e_version != EV_CURRENT)
>  >      return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-independent
> ELF magic"));
>  >
>  >    return GRUB_ERR_NONE;
>  > @@ -116,7 +132,11 @@ grub_elf_open (const char *name)
>  >    return elf;
>  >  }
>  >
>  > -
>  > +#define grub_be_to_halfXX grub_be_to_cpu16
>  > +#define grub_le_to_halfXX grub_le_to_cpu16
>  > +#define grub_be_to_wordXX grub_be_to_cpu32
>  > +#define grub_le_to_wordXX grub_le_to_cpu32
>  > +
>  >  /* 32-bit */
>  >  #define ehdrXX ehdr32
>  >  #define ELFCLASSXX ELFCLASS32
>  > @@ -127,7 +147,15 @@ grub_elf_open (const char *name)
>  >  #define grub_elf_is_elfXX grub_elf_is_elf32
>  >  #define grub_elfXX_load_phdrs grub_elf32_load_phdrs
>  >  #define ElfXX_Phdr Elf32_Phdr
>  > +#define ElfXX_Ehdr Elf32_Ehdr
>  >  #define grub_uintXX_t grub_uint32_t
>  > +#define grub_be_to_addrXX grub_be_to_cpu32
>  > +#define grub_le_to_addrXX grub_le_to_cpu32
>  > +#define grub_be_to_offXX grub_be_to_cpu32
>  > +#define grub_le_to_offXX grub_le_to_cpu32
>  > +#define grub_be_to_XwordXX grub_be_to_cpu32
>  > +#define grub_le_to_XwordXX grub_le_to_cpu32
>  > +#define grub_elfXX_check_endianess grub_elf32_check_endianess
>  >
>  >  #include "elfXX.c"
>  >
>  > @@ -140,9 +168,16 @@ grub_elf_open (const char *name)
>  >  #undef grub_elf_is_elfXX
>  >  #undef grub_elfXX_load_phdrs
>  >  #undef ElfXX_Phdr
>  > +#undef ElfXX_Ehdr
>  >  #undef grub_uintXX_t
>  > +#undef grub_be_to_addrXX
>  > +#undef grub_le_to_addrXX
>  > +#undef grub_be_to_offXX
>  > +#undef grub_le_to_offXX
>  > +#undef grub_be_to_XwordXX
>  > +#undef grub_le_to_XwordXX
>  > +#undef grub_elfXX_check_endianess
>  >
>  > -
>  >  /* 64-bit */
>  >  #define ehdrXX ehdr64
>  >  #define ELFCLASSXX ELFCLASS64
>  > @@ -153,6 +188,14 @@ grub_elf_open (const char *name)
>  >  #define grub_elf_is_elfXX grub_elf_is_elf64
>  >  #define grub_elfXX_load_phdrs grub_elf64_load_phdrs
>  >  #define ElfXX_Phdr Elf64_Phdr
>  > +#define ElfXX_Ehdr Elf64_Ehdr
>  >  #define grub_uintXX_t grub_uint64_t
>  > +#define grub_be_to_addrXX grub_be_to_cpu64
>  > +#define grub_le_to_addrXX grub_le_to_cpu64
>  > +#define grub_be_to_offXX grub_be_to_cpu64
>  > +#define grub_le_to_offXX grub_le_to_cpu64
>  > +#define grub_be_to_XwordXX grub_be_to_cpu64
>  > +#define grub_le_to_XwordXX grub_le_to_cpu64
>  > +#define grub_elfXX_check_endianess grub_elf64_check_endianess
>  >
>  >  #include "elfXX.c"
>  > diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
>  > index 1d09971..c14e071 100644
>  > --- a/grub-core/kern/elfXX.c
>  > +++ b/grub-core/kern/elfXX.c
>  > @@ -31,6 +31,39 @@ grub_elfXX_load_phdrs (grub_elf_t elf)
>  >        return grub_errno;
>  >      }
>  >
>  > +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
>  > +  ElfXX_Phdr *phdr;
>  > +  for (phdr = elf->phdrs;
>  > +       phdr && phdr < (ElfXX_Phdr *) elf->phdrs +
> elf->ehdr.ehdrXX.e_phnum;
>  > +       phdr++)
>  > +    {
>  > +      if (elf->ehdr.ehdrXX.e_ident[EI_DATA] == ELFDATA2LSB)
>  > +        {
>  > +          phdr->p_type = grub_le_to_wordXX (phdr->p_type);
>  > +          phdr->p_flags = grub_le_to_wordXX (phdr->p_flags);
>  > +          phdr->p_offset = grub_le_to_offXX (phdr->p_offset);
>  > +          phdr->p_vaddr = grub_le_to_addrXX (phdr->p_vaddr);
>  > +          phdr->p_paddr = grub_le_to_addrXX (phdr->p_paddr);
>  > +          phdr->p_filesz = grub_le_to_XwordXX (phdr->p_filesz);
>  > +          phdr->p_memsz = grub_le_to_XwordXX (phdr->p_memsz);
>  > +          phdr->p_align = grub_le_to_XwordXX (phdr->p_align);
>  > +        }
>  > +#if !defined(GRUB_CPU_WORDS_BIGENDIAN) && 0
> &&0. Really. AFAICT this breaks bigendian elf support. Please test your
> patch with be kernel

I built and tested in both endianess. No problem found. Will double 
check it.

>  > +      else if (elf->ehdr.ehdrXX.e_ident[EI_DATA] == ELFDATA2MSB)
>  > +        {
>  > +          phdr->p_type = grub_be_to_wordXX (phdr->p_type);
>  > +          phdr->p_flags = grub_be_to_wordXX (phdr->p_flags);
>  > +          phdr->p_offset = grub_be_to_offXX (phdr->p_offset);
>  > +          phdr->p_vaddr = grub_be_to_addrXX (phdr->p_vaddr);
>  > +          phdr->p_paddr = grub_be_to_addrXX (phdr->p_paddr);
>  > +          phdr->p_filesz = grub_be_to_XwordXX (phdr->p_filesz);
>  > +          phdr->p_memsz = grub_be_to_XwordXX (phdr->p_memsz);
>  > +          phdr->p_align = grub_be_to_XwordXX (phdr->p_align);
>  > +        }
>  > +#endif /* !defined(GRUB_CPU_WORDS_BIGENDIAN) && 0 */
>  > +    }
>  > +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
>  > +
>  >    return GRUB_ERR_NONE;
>  >  }
>  >
>  > @@ -154,3 +187,46 @@ grub_elfXX_load (grub_elf_t elf, const char
> *filename,
>  >
>  >    return grub_errno;
>  >  }
>  > +
>  > +#if defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275)
>  > +void
>  > +grub_elfXX_check_endianess (grub_elf_t elf)
>  > +{
>  > +  ElfXX_Ehdr *e = &(elf->ehdr.ehdrXX);
>  > +
>  > +  if (e->e_ident[EI_DATA] == ELFDATA2LSB)
>  > +    {
>  > +      e->e_type = grub_le_to_halfXX (e->e_type);
> This is more than just check. Please reflect it in function name for
> code readability.

Agree. Maybe grub_check_convert_endianess?

>  > +      e->e_machine = grub_le_to_halfXX (e->e_machine);
>  > +      e->e_version = grub_le_to_wordXX (e->e_version);
>  > +      e->e_entry = grub_le_to_addrXX (e->e_entry);
>  > +      e->e_phoff = grub_le_to_offXX (e->e_phoff);
>  > +      e->e_shoff = grub_le_to_offXX (e->e_shoff);
>  > +      e->e_flags = grub_le_to_wordXX (e->e_flags);
>  > +      e->e_ehsize = grub_le_to_halfXX (e->e_ehsize);
>  > +      e->e_phentsize = grub_le_to_halfXX (e->e_phentsize);
>  > +      e->e_phnum = grub_le_to_halfXX (e->e_phnum);
>  > +      e->e_shentsize = grub_le_to_halfXX (e->e_shentsize);
>  > +      e->e_shnum = grub_le_to_halfXX (e->e_shnum);
>  > +      e->e_shstrndx = grub_le_to_halfXX (e->e_shstrndx);
>  > +    }
>  > +#if !defined(GRUB_CPU_WORDS_BIGENDIAN) && 0
>  > +  else if (e->e_ident[EI_DATA] == ELFDATA2MSB)
>  > +    {
>  > +      e->e_type = grub_be_to_halfXX (e->e_type);
>  > +      e->e_machine = grub_be_to_halfXX (e->e_machine);
>  > +      e->e_version = grub_be_to_wordXX (e->e_version);
>  > +      e->e_entry = grub_be_to_addrXX (e->e_entry);
>  > +      e->e_phoff = grub_be_to_offXX (e->e_phoff);
>  > +      e->e_shoff = grub_be_to_offXX (e->e_shoff);
>  > +      e->e_flags = grub_be_to_wordXX (e->e_flags);
>  > +      e->e_ehsize = grub_be_to_halfXX (e->e_ehsize);
>  > +      e->e_phentsize = grub_be_to_halfXX (e->e_phentsize);
>  > +      e->e_phnum = grub_be_to_halfXX (e->e_phnum);
>  > +      e->e_shentsize = grub_be_to_halfXX (e->e_shentsize);
>  > +      e->e_shnum = grub_be_to_halfXX (e->e_shnum);
>  > +      e->e_shstrndx = grub_be_to_halfXX (e->e_shstrndx);
>  > +    }
>  > +#endif /* !defined(GRUB_CPU_WORDS_BIGENDIAN) && 0 */
>  > +}
>  > +#endif /* defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275) */
>  > --
>  > 2.1.0
>  >
>  >
>  > _______________________________________________
>  > Grub-devel mailing list
>  > Grub-devel@gnu.org <mailto:Grub-devel@gnu.org>
>  > https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


-- 
Paulo Flabiano Smorigo
IBM Linux Technology Center