Re: [Qemu-devel] [PATCH] mips: Fix "Unexpected FPU mode"

[Aleksandar Markovic <aleksandar.m.mail@gmail.com>]

On Wed, Apr 17, 2019 at 9:50 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>
> In load_elf_binary, struct image_info interp_info is used without being
> properly initialized.  One result is that when the ELF's program header
> doesn't contain an entry for the ABI flags, then the value of the struct
> image_info's fp_abi field is set to whatever happened to be in stack
> memory at the time.
>
> This patch both sanitizes interp_info and initializes fp_abi for
> TARGET_MIPS to MIPS_ABI_FP_UNKNOWN so that when we don't know the FP
> ABI, we don't just blow up.  Currently, this bug is a complete stopper
> for some MIPS binaries.
>
> ***PLEASE NOTE***
> There may be other bugs as a result of struct image_info interp_info
> fields not being properly initialized -- this patch only addresses the
> fp_abi field.  I reccomend somebody who knows the code better than I
> audit this function and the whole of that execution path.
>
> Fixes bug #1825002 and affects 3.1.0 and 4.x, reccomend backporting to
> 3.1.0.
>
> Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
> ---

Daniel, not knowing that you already send this patch, I included it in another
series (with different title and commit message, but the same content):

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03813.html

Please let's track this patch there.

I will change the commit message to bring it closer to the yours version
in the next version of the series, and I will review the patch from MIPS
point of view.

Just advice for the future: Before sending patches to qemu-devel,
check what are maintainers for the applicable code. There is even a
script for that: <qemu-root>/scripts/get_maintainers.pl

There are also other rules and conventiones, and all of them are
mentioned on the page "How to submit a patch" on QEMU web site.

But, in any case, many thanks for discovering and reporting the bug,
and even devising the fix!

Yours,
Aleksandar

>  linux-user/elfload.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a26021f8..7f09d572a2 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
>      char *elf_interpreter = NULL;
>      char *scratch;
>
> +    memset(&interp_info, 0, sizeof(interp_info));
> +#ifdef TARGET_MIPS
> +    interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
> +#endif
> +
>      info->start_mmap = (abi_ulong)ELF_START_MMAP;
>
>      load_elf_image(bprm->filename, bprm->fd, info,
> --
> 2.19.2
>
>