Re: [PATCH v2] linux-user: detect mismatched ELF ABI in qemu-mips[n32][el]

[Laurent Vivier <laurent@vivier.eu>]

Le 23/08/2020 à 12:17, Carlo Marcelo Arenas Belón a écrit :
> MIPS provides 2 ILP32 ABIs, and therefore 4 possible qemu-mips binaries
> with 2 pairs using the same endianess and bitness.
> 
> This could lead to an O32 image loading in the N32 binary or vice versa
> and in cryptic errors (if lucky that the CPU doesn't match the FPU used)
> like :
> 
>   qemu: Unexpected FPU mode       (o32 ELF loaded to qemu-mipsn32[el])
>   ELF binary's NaN mode not supported by CPU    (n32 -> qemu-mips[el])
> 
> Add an ABI check macro that could be used while checking the ELF header
> that relies in the ABI2 flag to identify n32 binaries and abort instead
> early with a more descriptive error :
> 
>   Invalid ELF image for this architecture
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> Changes since v1:
> - Use the provided definition from include/elf.h (per Laurent)
> - Abort instead of warning (per Laurent, not using a custom error though)
> - Expand the check to all other combinations (per Aleksandar)
> 
>  linux-user/elfload.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index fe9dfe795d..69936dcd45 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -918,6 +918,12 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
>  
>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
>  
> +#ifdef TARGET_ABI_MIPSN32
> +#define elf_check_abi(x) ((x) & EF_MIPS_ABI2)
> +#else
> +#define elf_check_abi(x) (!((x) & EF_MIPS_ABI2))
> +#endif
> +
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
>  {
> @@ -1487,6 +1493,10 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
>  #define elf_check_arch(x) ((x) == ELF_ARCH)
>  #endif
>  
> +#ifndef elf_check_abi
> +#define elf_check_abi(x) (1)
> +#endif
> +
>  #ifndef ELF_HWCAP
>  #define ELF_HWCAP 0
>  #endif
> @@ -1644,6 +1654,7 @@ static bool elf_check_ident(struct elfhdr *ehdr)
>  static bool elf_check_ehdr(struct elfhdr *ehdr)
>  {
>      return (elf_check_arch(ehdr->e_machine)
> +            && elf_check_abi(ehdr->e_flags)
>              && ehdr->e_ehsize == sizeof(struct elfhdr)
>              && ehdr->e_phentsize == sizeof(struct elf_phdr)
>              && (ehdr->e_type == ET_EXEC || ehdr->e_type == ET_DYN));
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>