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

[Andrei Borzenkov]

В Fri, 24 Jul 2015 21:48:22 +0300
Andrei Borzenkov <address@hidden> пишет:

> В Thu, 23 Jul 2015 11:10:36 -0300
> Paulo Flabiano Smorigo <address@hidden> пишет:
> 
> > 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.
> > 
> 
> Could you add a bit more verbose description that it currently applies
> to ppc only when committing.
> 
> > 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"));
> 
> Other places are using "invalid arch-dependent ELF magic" in this case,
> it is better to reuse translation string. Otherwise fine for me. 
> 

BTW this duplicates check in grub-core/loader/powerpc/ieee1275/linux.c
which probably can then be removed then.