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

[Libvir] Fix buffer overflow in dumping XML



The new bufferContentAndFree() method used for the QEMU daemon rellocs the
buffer size down to release memory held by the buffer which was never used
for any data. Unfortunately it reallocs it 1 byte too small, so later uses
of strlen()/strcpy() either magically work, or randomly append gargage or
crash the daemon depending on the phase of the moon :-) Re-allocing the
buffer to relase a few bytes memory isn't really an optimization since the
caller is going to free the entire block a very short while later, so this
patch simply removes the realloc call.

As an aside, the virBuffer functions in src/xml.c and the buffer functions
in qemud/buf.c are both flawed wrt to the way they call the Grow method. 
The method expects the len parameter to be extra bytes needed, but several
of the callers pass in the total desired length, so it allocates too much
memory. There are various other non-fatal flaws which need to be cleaned
up in this code, but the attached patch just focuses on the current fatal
buffer overflow for now.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
Index: qemud/buf.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/buf.c,v
retrieving revision 1.1
diff -u -r1.1 buf.c
--- qemud/buf.c	15 Mar 2007 17:30:04 -0000	1.1
+++ qemud/buf.c	21 Mar 2007 15:01:58 -0000
@@ -118,18 +118,7 @@
 char *
 bufferContentAndFree (bufferPtr buf)
 {
-    char *content;
-
-    content = buf->content;
-
-    /* Try to reduce the size of the block, but if it fails, it doesn't
-     * matter.
-     */
-    if (buf->use < buf->size) {
-        char *old_content = content;
-        content = realloc (content, buf->use);
-        content = content ? content : old_content;
-    }
+    char *content = buf->content;
 
     free (buf);
     return content;

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