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

[Andrei Borzenkov]

В Fri, 24 Jul 2015 21:00:50 +0200
"Vladimir 'phcoder' Serbinenko" <address@hidden> пишет:

> Le 23 juil. 2015 16:11, "Paulo Flabiano Smorigo" <
> address@hidden> 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 <address@hidden>
> > ---
> >  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

Not really. This is noop as code is under __powerpc__ which is BE
always, so there is nothing to do and code compiles out. This is just
placeholder if we ever need similar conversion from BE to LE. I'm fine
with removing it altogether to avoid confusion and adding if needed.

> > +      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.
> > +      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
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel