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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)