[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size



On 08/16/2010 03:37 AM, Soren Hansen wrote:
> 
> And I get this output:
> 11:28:14.602: debug : umlMonitorCommand:702 : Run command 'config con0'
> 11:28:14.602: debug : umlMonitorCommand:737 : res.error: 5, res.extra: 0, res.length: 4096, res.data: 
> 11:28:14.602: debug : umlMonitorCommand:740 : nbytes: 28
> 11:28:14.602: debug : umlMonitorCommand:742 : res.error: 0, res.extra: 0, res.length: 16, res.data: pts:/dev/pts/31
> 11:28:14.602: debug : umlMonitorCommand:771 : Command reply is 'pts:/dev/pts/31'
> 
> So the size of the response datagram isn't sizeof(res) as the check in
> uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) +
> sizeof(res.length) + res.length.

I agree with this analysis.  In other words, the check should be more
like this (two conditions - did we get enough bytes to even have a valid
res.length, and did we get enough bytes to match with what res.length
stated):

if (nbytes < offsetof(struct monitor_request, data) ||
    nbytes < res.length + offsetof(struct monitor_request, data))
    incomplete reply

instead of the two current but incorrect checks:

if (nbytes < sizeof ret) /* incorrectly true except when
res.length==MONITOR_BUFLEN */
  incomplete reply;
if (sizeof res.data < res.length) /* res.length of MONITOR_BUFLEN+1 does
not trigger this check, but leads to a buffer overflow in subsequent
memmove */
  invalid length;

But before I write such a patch, I'm going to look in more details at
your other reply.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]