[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] linux-user: fix preadv/pwritev offsets
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH v3] linux-user: fix preadv/pwritev offsets |
Date: |
Thu, 5 Apr 2018 18:33:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Le 05/04/2018 à 18:27, Max Filippov a écrit :
> 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.
I don't think it can happen, because "long long" is always 64bit, so it
fits in two 32bit (long is 32bit on 32bit, 64bit on 64bit).
Thanks,
Laurent