Re: [Qemu-devel] [PATCH 4/9] linux-user: Fix sendrecvmsg() with QEMU_GUEST_BASE

[Alexander Graf]

On 06.07.2013, at 12:42, Peter Maydell wrote:

> On 6 July 2013 01:36, Alexander Graf <address@hidden> wrote:
>> While looking for cmsg entries, we want to compare guest pointers to see
>> whether we're at the end of the passed in array.
>> 
>> However, what we really do is we compare our in-use host pointer with the
>> to-be-the-end guest pointer. This comparison is obviously bogus.
>> 
>> Change the comparison to compare guest pointer with guest pointer.
>> 
>> Signed-off-by: Alexander Graf <address@hidden>
>> ---
>> linux-user/syscall_defs.h |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 92c01a9..8b06a19 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -214,7 +214,7 @@ __target_cmsg_nxthdr (struct target_msghdr *__mhdr, 
>> struct target_cmsghdr *__cms
>> 
>>   __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg
>>                                     + TARGET_CMSG_ALIGN 
>> (tswapal(__cmsg->cmsg_len)));
>> -  if ((unsigned long)((char *)(__ptr+1) - (char 
>> *)(size_t)tswapal(__mhdr->msg_control))
>> +  if ((unsigned long)((char *)(h2g(__ptr+1)) - (char 
>> *)(size_t)tswapal(__mhdr->msg_control))
>>> tswapal(__mhdr->msg_controllen))
>>     /* No more entries.  */
>>     return (struct target_cmsghdr *)0;
> 
> I don't think this is right. The passed in __cmsg (and thus the
> __ptr we calculate) isn't a guest address -- it's the address
> we get back from calling lock_user() on a guest address.

... which makes it a host address we want to convert into guest address space, 
so we can do a guest <-> guest comparison.

> That can't be validly compared with anything except another
> address derived by arithmetic from the same lock_user()
> return value (because if DEBUG_REMAP is defined then the

Ah, ok. I didn't know about that debug flag. That might break, yes.

> value you get back from lock_user() is the result of calling
> malloc()). What we ought to be comparing __ptr+1 against
> is not tswapal(__mhdr->msg_control) but the initial value
> of target_cmsg returned from lock_user().

Ok :).


Alex