[PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF

Carlo Marcelo Arenas Belón posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200821222830.91463-1-carenas@gmail.com
Maintainers: Jiaxun Yang <jiaxun.yang@flygoat.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
linux-user/elfload.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF
Posted by Carlo Marcelo Arenas Belón 3 years, 8 months ago
While technically compatible will (depending on the CPU) hopefully fail
later with a cryptic error:

  qemu: Unexpected FPU mode

Provide an earlier hint of what the problem might be by detecting if the
binary might not be using the n32 ABI and print a warning.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 linux-user/elfload.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index fe9dfe795d..64c3921cd9 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2390,6 +2390,13 @@ static void load_elf_image(const char *image_name, int image_fd,
     if (!elf_check_ehdr(ehdr)) {
         goto exit_errmsg;
     }
+#ifdef TARGET_ABI_MIPSN32
+/* from arch/mips/include/asm/elf.h */
+#define EF_MIPS_ABI2 0x00000020
+    if (!(ehdr->e_flags & EF_MIPS_ABI2)) {
+        fprintf(stderr, "warning: ELF binary missing n32 flag\n");
+    }
+#endif
 
     i = ehdr->e_phnum * sizeof(struct elf_phdr);
     if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) {
-- 
2.28.0.213.gdd9653da77


Re: [PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF
Posted by Laurent Vivier 3 years, 8 months ago
Le 22/08/2020 à 00:28, Carlo Marcelo Arenas Belón a écrit :
> While technically compatible will (depending on the CPU) hopefully fail
> later with a cryptic error:
> 
>   qemu: Unexpected FPU mode
> 
> Provide an earlier hint of what the problem might be by detecting if the
> binary might not be using the n32 ABI and print a warning.
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  linux-user/elfload.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index fe9dfe795d..64c3921cd9 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2390,6 +2390,13 @@ static void load_elf_image(const char *image_name, int image_fd,
>      if (!elf_check_ehdr(ehdr)) {
>          goto exit_errmsg;
>      }
> +#ifdef TARGET_ABI_MIPSN32
> +/* from arch/mips/include/asm/elf.h */
> +#define EF_MIPS_ABI2 0x00000020

This is already defined in include/elf.h

> +    if (!(ehdr->e_flags & EF_MIPS_ABI2)) {
> +        fprintf(stderr, "warning: ELF binary missing n32 flag\n");

I think an exit would be more appropriate:

    errmsg = "warning: ELF binary missing n32 flag";
    goto exit_errmsg;

> +    }
> +#endif
>  
>      i = ehdr->e_phnum * sizeof(struct elf_phdr);
>      if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) {
> 

CC'ing mips maintainers (and Maciej as he sent a patch on the N32 topic
2 years ago...)

Thanks,
Laurent


Re: [PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF
Posted by Aleksandar Markovic 3 years, 8 months ago
On Saturday, August 22, 2020, Laurent Vivier <laurent@vivier.eu> wrote:

> Le 22/08/2020 à 00:28, Carlo Marcelo Arenas Belón a écrit :
> > While technically compatible will (depending on the CPU) hopefully fail
> > later with a cryptic error:
> >
> >   qemu: Unexpected FPU mode
> >
> > Provide an earlier hint of what the problem might be by detecting if the
> > binary might not be using the n32 ABI and print a warning.
> >
> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> > ---
> >  linux-user/elfload.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index fe9dfe795d..64c3921cd9 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -2390,6 +2390,13 @@ static void load_elf_image(const char
> *image_name, int image_fd,
> >      if (!elf_check_ehdr(ehdr)) {
> >          goto exit_errmsg;
> >      }
> > +#ifdef TARGET_ABI_MIPSN32
> > +/* from arch/mips/include/asm/elf.h */
> > +#define EF_MIPS_ABI2 0x00000020
>
> This is already defined in include/elf.h
>
> > +    if (!(ehdr->e_flags & EF_MIPS_ABI2)) {
> > +        fprintf(stderr, "warning: ELF binary missing n32 flag\n");
>
> I think an exit would be more appropriate:
>
>     errmsg = "warning: ELF binary missing n32 flag";
>     goto exit_errmsg;
>

Carlo, Laurent,

I agree with both Laurent's remarks above.

At the same time, I also support the spirit of Carlo's patch - but please,
Carlo, if possible, send the v2 that is going to address Laurent's concerns.

Moreover, Carlo, please consider doing more cleanup in this area. For
example, is the case "attempt passing n32 executable to non-n32 qemu"
handled in a user-friendly manner? What about "passing 64-bit executable to
32-bit qemu"? What about opposite endians? Are all user visible messages
clear and appropriate? Could you do a thourough analysis in this area?
There are 36 combinations (6x6, given 6 mips supported ABIs).

I would prefer a complete solution for all these mips combinations
"executable vs qemu" in linux user. Perhaps you can send a series of
patches on this topic, this one being, let's say, just the first one in
that series.

Yours,
Aleksandar




> > +    }
> > +#endif
> >
> >      i = ehdr->e_phnum * sizeof(struct elf_phdr);
> >      if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) {
> >
>
> CC'ing mips maintainers (and Maciej as he sent a patch on the N32 topic
> 2 years ago...)
>
> Thanks,
> Laurent
>
>
Re: [PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF
Posted by Carlo Arenas 3 years, 8 months ago
The differences of bit width and endianness are already being taken
care of by the current code.

The differences in ABI for ILP32 are the only ones missing and so the
new version[1] (sorry, not a series since it makes sense as a single
change anyway) should be enough to fix any inconsistencies.

With this change, all linux-user versions will show the same
consistent error when asked to load an incorrect (wrong architecture,
endianness or bit width) binary

Carlo

[1] https://patchwork.ozlabs.org/project/qemu-devel/patch/20200823101703.18451-1-carenas@gmail.com/