Re: [RFC v4 08/21] vfio-user: define socket receive functions

[John Johnson]

> On Feb 4, 2022, at 4:42 AM, Thanos Makatos <thanos.makatos@nutanix.com> wrote:
> 
>> -----Original Message-----
>> From: Qemu-devel <qemu-devel-
>> bounces+thanos.makatos=nutanix.com@nongnu.org> On Behalf Of Thanos
>> Makatos
>> Sent: 03 February 2022 21:54
>> To: John Johnson <john.g.johnson@oracle.com>; qemu-devel@nongnu.org
>> Subject: RE: [RFC v4 08/21] vfio-user: define socket receive functions
>> 
>> 
>> 
>>> -----Original Message-----
>>> From: Qemu-devel <qemu-devel-
>>> bounces+thanos.makatos=nutanix.com@nongnu.org> On Behalf Of John
>>> Johnson
>>> Sent: 12 January 2022 00:44
>>> To: qemu-devel@nongnu.org
>>> Subject: [RFC v4 08/21] vfio-user: define socket receive functions
>>> 
>>> +    }
>>> +
>>> +    msgleft = hdr.size - sizeof(hdr);
>>> +    while (msgleft > 0) {
>>> +        ret = qio_channel_read(proxy->ioc, data, msgleft, &local_err);
>>> +
>>> +        /* error or would block */
>>> +        if (ret < 0) {
>>> +            goto fatal;
>>> +        }
>> 
>> IIUC qio_channel_read() ends up calling qio_channel_socket_readv() which can
>> return QIO_CHANNEL_ERR_BLOCK (-2). The if will be taken so local_err is NULL
>> and that causes a segfault when error_report_err(local_err) is called before
>> returning from this function.
> 
> In fact, don't we need to continue if qio_channel_read() returns 
> QIO_CHANNEL_ERR_BLOCK and only fail if it returns -1?
> 
>> 
>>> +
>>> +        msgleft -= ret;
>>> +        data += ret;
>>> +    }
>>> +


        I can’t loop indefinitely, as a malicious server could cause the 
receiver to loop
continuously if it sends a packet with a header length greater than the packet 
length.

        If large messages are being fragmented by the socket code, then I think 
I’ll need
to change the packet parser to able to reassemble them.

                                                                        JJ