Re: [PATCH qemu 1/2] Don't reverse bFLT endianess when not needed

[Wojtek Kaniewski <wojtekka@toxygen.net>]

On Thu, 2025-07-17 at 08:48 +0200, Laurent Vivier wrote:
> Le 16/07/2025 à 21:24, ~wojtekka a écrit :
> > From: Wojtek Kaniewski <wojtekka@toxygen.net>
> > 
> > bFLT format is big-endian. get_user_ual() returns host value so for
> > little-endian target and little-endian host it's necessary to
> > reverse
> > words using ntohl(). For big-endian targets we end up with
> > incorrect
> > endianess:
> > 
> >      $ qemu-m68k-static ./test
> >      BINFMT_FLAT: reloc outside program 0x801f0000 (0 -
> > 0x41f0/0x1e40)
> >      Aborted (core dumped)
> > 
> > For comparison the output of `flthdr` follows:
> > 
> >      $ m68k-elf-flthdr -P ./test
> >      ./test
> >          Magic:        bFLT
> >          Rev:          4
> >          Build Date:   Tue Jul 15 23:02:00 2025
> >          Entry:        0x44
> >          Data Start:   0x1e80
> >          Data End:     0x205c
> >          BSS End:      0x40a0
> >          Stack Size:   0x1000
> >          Reloc Start:  0x205c
> >          Reloc Count:  0x17
> >          Flags:        0x2 ( Has-PIC-GOT )
> >          Relocs:
> >          #	reloc      (  address )	data
> >          0	0x00001f80 (0x00001f80)	3c200000
> >                    ^^^^^^^^
> >          1	0x00001f84 (0x00001f84)	3c300000
> >          2	0x00001f88 (0x00001f88)	3c200000
> >          ...
> > 
> > Signed-off-by: Wojtek Kaniewski <wojtekka@toxygen.net>
> > ---
> >   linux-user/flatload.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> > index 4beb3ed1b9..afaff4ac44 100644
> > --- a/linux-user/flatload.c
> > +++ b/linux-user/flatload.c
> > @@ -413,7 +413,9 @@ static int load_flat_file(struct linux_binprm *
> > bprm,
> >                  relocated first).  */
> >               if (get_user_ual(relval, reloc + i *
> > sizeof(abi_ulong)))
> >                   return -EFAULT;
> > +#if !TARGET_BIG_ENDIAN
> >               relval = ntohl(relval);
> > +#endif
> >               if (flat_set_persistent(relval, &persistent))
> >                   continue;
> >               addr = flat_get_relocate_addr(relval);
> 
> I think the ntohl() should be removed totally and
> flat_set_persistent() 
> should use put_user_ual().
> 
> Could you test this?

flat_set_persistent() is just a no-op macro copied from Linux kernel
implementation and has been removed from the upstream a couple of years
ago [1]. The actual address is relocated by flat_get_relocate_addr() a
line below so it needs to be in host-endian. Dropping ntohl() would
break it for little-endian targets because bFLT by definition is big-
endian.

Is there a macro similar to get_user_ual() but without any endianess
conversion? The closest thing I found was ldl_p(), but I assume it's
not the best idea due to lack of locking.

(And sorry for such a late response. I haven't used mailing lists for
years and it took me way too long to get my old e-mail account to work
properly.)

Regards,
Wojtek

[1]
https://github.com/torvalds/linux/commit/2f3196d49b1e10f1d4bc64cce00dc95fde2b0ce1