Re: [PATCH 3/4] linux-user: Fix mmap2() syscall on 32-bit targets to allow file mapping beyond 4GB

[Helge Deller]

On 7/7/23 21:47, Richard Henderson wrote:
> On 7/7/23 14:19, Helge Deller wrote:
> > The mmap2() syscall allows 32-bit guests to specify the offset into a
> > file in page units (instead of bytes, as done by mmap(2)).
> > On physical machines this allows 32-bit applications to map such parts
> > of large files which are stored beyond the 4GB limit.
> >
> > Allow the same behaviour when emulating 32-bit guests with qemu.
> >
> > For that switch the mmap2() function to always take an abi_ullong
> > (64-bit) offset parameter for target_mmap() and mmap_frag() to avoid an
> > arithmetical overflow when shifing a 32-bit offset parameter by
> > 12 bits (=PAGE_SHIFT) and thus possibly overflow the abi_ulong (32-bit)
> > type.
> >
> > Signed-off-by: Helge Deller<deller@gmx.de>
> > ---
> >   linux-user/mmap.c      | 9 +++++----
> >   linux-user/syscall.c   | 2 +-
> >   linux-user/user-mmap.h | 2 +-
> >   3 files changed, 7 insertions(+), 6 deletions(-)
>
> https://patchew.org/QEMU/20230630132159.376995-1-richard.henderson@linaro.org/20230630132159.376995-12-richard.henderson@linaro.org/
>
> Wherein I use the host off_t (which must be 64-bits).

I like your patch.
But wouldn't it be better to use off64_t instead of off_t just to make
clear that this is a 64bit int?

And this part:
-                          arg5, arg6 << MMAP_SHIFT);
+                          arg5, (off_t)(abi_ulong)arg6 << MMAP_SHIFT);
maybe should become (with brackets): ?
+                          arg5, ((off64_t)(abi_ulong)arg6) << MMAP_SHIFT);

In any case I'm fine if your or my patch could be appled.

Helge