Re: [Qemu-devel] [PATCH v3] linux-user: fix preadv/pwritev offsets

[Max Filippov]

On Thu, Apr 5, 2018 at 8:52 AM, Laurent Vivier <address@hidden> wrote:
> Why don't you try to de-construct then re-construct the offset?

It would require 128-bit arithmetic on 64-bit host.

> Kernel commit
>   601cc11d054a "Make non-compat preadv/pwritev use native register size"
> is interesting.
>
> static inline abi_llong target_pos_from_hilo(abi_ulong high,
>                                              abi_ulong low)
> {
> #define TARGET_HALF_LONG_BITS (TARGET_LONG_BITS / 2)
>    return (((abi_llong)high << TARGET_HALF_LONG_BITS) <<
>                              TARGET_HALF_LONG_BITS) | low;
> }
>
> #define HOST_HALF_LONG_BITS (HOST_LONG_BITS / 2)
>
>  abi_llong pos = target_pos_from_hilo(arg4, arg5);
>  ret = get_errno(safe_preadv(arg1, vec, arg3,
>           pos,
>           (pos >> HOST_HALF_LONG_BITS) >> HOST_HALF_LONG_BITS));
>
> It looks simpler, but perhaps I miss something
> (it's just cut&paste, I don't pretend to understand that code...)?

If we decide that host unsigned long long is wide enough to represent
meaningful file positions then this function may be simplified to
something like

unsigned long long off = (unsigned long long)tlow |
((unsigned long long)thigh << (TARGET_LONG_BITS / 2)) << (TARGET_LONG_BITS / 2);
*hlow = (unsigned long)off;
*hhigh = (unsigned long)((off >> (HOST_LONG_BITS / 2)) >> (HOST_LONG_BITS / 2));

There's also a bug that the target may specify an offset not representable
on the host, that will get truncated and point to a different location in the
file, possibly resulting in data corruption.

-- 
Thanks.
-- Max