[libvirt] [PATCH] uml: avoid crash on partial read

[Jim Meyering]

Daniel Veillard wrote:
> On Tue, Mar 02, 2010 at 05:16:05PM -0700, Eric Blake wrote:
>> Coverity detected a potential dereference of uninitialized memory
>> if recvfrom got cut short.
>>
>> * src/uml/uml_driver.c (umlMonitorCommand): Validate complete read
>> prior to dereferencing res.
>> ---
>>
>> The patch borrows ideas from macvtap.c, the only other file in
>> libvirt that currently uses recvfrom.
>>
>> I did not analyze whether this is a security hole, where a
>> malicious UDP packet could intentionally force the dereferencing
>> of uninitialized memory to misbehave in a controlled manner.
>>
>>  src/uml/uml_driver.c |   14 ++++++++++++--
>>  1 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
>> index bbea429..eec239f 100644
>> --- a/src/uml/uml_driver.c
>> +++ b/src/uml/uml_driver.c
>> @@ -733,14 +733,24 @@ static int umlMonitorCommand(virConnectPtr conn,
>>      }
>>
>>      do {
>> +        ssize_t nbytes;
>>          addrlen = sizeof(addr);
>> -        if (recvfrom(priv->monitor, &res, sizeof res, 0,
>> -                     (struct sockaddr *)&addr, &addrlen) < 0) {
>> +        nbytes = recvfrom(priv->monitor, &res, sizeof res, 0,
>> +                          (struct sockaddr *)&addr, &addrlen) < 0;
>> +        if (nbytes < 0) {
>> +            if (errno == EAGAIN || errno == EINTR)
>> +                continue;
>>              virReportSystemError(errno,
>>                                   _("cannot read reply %s"),
>>                                   cmd);
>>              goto error;
>>          }
>> +        if (nbytes < sizeof res) {
>> +            virReportSystemError(errno,
>> +                                 _("incomplete reply %s"),
>> +                                 cmd);
>> +            goto error;
>> +        }
>>
>>          if (VIR_REALLOC_N(retdata, retlen + res.length) < 0) {
>>              virReportOOMError();
>
>   ACK, looks fine !

I pushed this, and then (sorry I missed it the first time)
noticed that we'd rather avoid that new use of errno.
errno is not defined for a shorter-than-expected read, so including
strerror(errno) in the diagnostic would be misleading.