[Qemu-devel] [PATCH] Fix for a malloc heap corruption problem in the slirp network code

[Juergen Keil <jk@tools.de>]

Compiling inside a NetBSD 1.5 qemu guest OS (source files are located
on an NFS filesystem mounted from the Solaris host OS) crashes qemu
with a malloc heap corruption error, when the slirp user mode
networking code is in use.


Solaris' umem malloc library reports that a block freed by a call to
realloc (from function m_inc()) is still being used, when it
shouldn't:

% mdb i386-softmmu/qemu core
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::umem_status
Status:         ready and active
Concurrency:    2
Logs:           transaction=64k (inactive)
Message buffer:
umem allocator: boundary tag corrupted
bcp ^ bxstat = 2c694858, should be f4eef4ee
buffer=1977f000  bufctl=19713340  cache: umem_alloc_8192
previous transaction on buffer 1977f000:
thread=1  time=T-0.009171995  slab=8f9b2c0  cache: umem_alloc_8192
libumem.so.1'?? (0xd1f99c49)
libumem.so.1'umem_cache_free+0x3f
libumem.so.1'umem_free+0xd5
libumem.so.1'?? (0xd1f97c05)
libumem.so.1'free+0x14
libumem.so.1'realloc+0x66
qemu'm_inc+0x26
qemu'm_cat+0x4b
qemu'ip_reass+0x321
qemu'ip_input+0x261
qemu'slirp_input+0x6f
qemu'?? (0x8065055)
qemu'qemu_send_packet+0x13
qemu'?? (0x807d116)
qemu'cpu_outb+0x1b
umem: heap corruption detected
stack trace:
libumem.so.1'?? (0xd1f96099)
libumem.so.1'?? (0xd1f98b1c)
libumem.so.1'?? (0xd1f99ad2)
libumem.so.1'umem_cache_alloc+0xe1
libumem.so.1'umem_alloc+0x3f
libumem.so.1'malloc+0x23
qemu'm_inc+0x41
qemu'm_cat+0x4b
qemu'ip_reass+0x321
qemu'ip_input+0x261
qemu'slirp_input+0x6f
qemu'?? (0x8065055)
qemu'qemu_send_packet+0x13
qemu'?? (0x807d116)
qemu'cpu_outb+0x1b
qemu'code_gen_buffer+0x1103bd
qemu'main_loop+0x22
qemu'main+0x10ce
qemu'_start+0x5d

>


Using the "electric fence" memory allocator, the location of the data
corruption can be narrowed down to the destination address in the memcpy
call in slirp/mbuf.c, function m_cat():

    void
    m_cat(m, n)
	register struct mbuf *m, *n;
    {
	/*
	 * If there's no room, realloc
	 */
	if (M_FREEROOM(m) < n->m_len)
		m_inc(m,m->m_size+MINCSIZE);
	
	memcpy(m->m_data+m->m_len, n->m_data, n->m_len);  <<<< this memcpy 
							  corrupts the malloc
							  heap
        ....
    }


The problem is apparently in m_inc(), when an mbuf with an external
data buffer is resized.  After resizing the mbuf, the m_data member
still points into the old buffer, before is was reallocated.  For some
reason, the code to re-adjust the m_data pointer is present in the M_EXT
case in m_inc(), but is commented out. (With a bit of luck, realloc
might be able to adjust the size of the memory block in place; but
slirp shouldn't rely on this)

Fix: Adjust mbuf->m_data after a realloc of the external data buffer

% gdiff -u qemu-cvs/slirp/mbuf.c qemu-debug/slirp/mbuf.c
--- qemu-cvs/slirp/mbuf.c       2004-04-22 02:10:47.000000000 +0200
+++ qemu-debug/slirp/mbuf.c     2005-05-16 11:24:14.557567000 +0200
@@ -146,18 +146,19 @@
         struct mbuf *m;
         int size;
 {
+       int datasize;
+
        /* some compiles throw up on gotos.  This one we can fake. */
         if(m->m_size>size) return;

         if (m->m_flags & M_EXT) {
-         /* datasize = m->m_data - m->m_ext; */
+         datasize = m->m_data - m->m_ext;
          m->m_ext = (char *)realloc(m->m_ext,size);
 /*             if (m->m_ext == NULL)
  *                     return (struct mbuf *)NULL;
  */
-         /* m->m_data = m->m_ext + datasize; */
+         m->m_data = m->m_ext + datasize;
         } else {
-         int datasize;
          char *dat;
          datasize = m->m_data - m->m_dat;
          dat = (char *)malloc(size);