From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1DfDoo-0004HL-AS
	for qemu-devel@nongnu.org; Mon, 06 Jun 2005 05:22:51 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1DfDoj-0004GY-0L
	for qemu-devel@nongnu.org; Mon, 06 Jun 2005 05:22:46 -0400
Received: from [199.232.76.173] (helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1DfDjH-0003Rf-G9
	for qemu-devel@nongnu.org; Mon, 06 Jun 2005 05:17:08 -0400
Received: from [192.76.135.70] (helo=kurt.tools.de)
	by monty-python.gnu.org with esmtp
	(TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA:24) (Exim 4.34) id 1DfDcI-0002Qq-DB
	for qemu-devel@nongnu.org; Mon, 06 Jun 2005 05:09:55 -0400
Message-Id: <200506060906.j5696Q6f012427@imap3.tools.intra>
Date: Mon, 6 Jun 2005 11:06:26 +0200 (CEST)
From: Juergen Keil <jk@tools.de>
Subject: Re: [Qemu-devel] [PATCH] Fix for a malloc heap corruption problem in
	the slirp network code
MIME-Version: 1.0
Content-Type: TEXT/plain; charset=us-ascii
Content-MD5: pTGyYZzP4GNVewEQXJuCcQ==
Reply-To: Juergen Keil <jk@tools.de>, qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Fabrice Bellard <fabrice@bellard.org>, qemu-devel@nongnu.org


> Fabrice Bellard wrote
> 
> Juergen Keil wrote:
> > 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.
> > [...]
> 
> > 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);
> 
> First this code is incorrect : it increases the size by MINCSIZE which 
> can be smaller than the required size.

Yep.

The m_cat() code might work in the slirp environment because its only
use is to reassemble ip fragments, which have a max size (~1500 bytes) that
is always smaller than MINCSIZE (4096).

To make the code more robust, it won't hurt to make sure the size of the
free room on the "m" mbuf is at least "n->m_len" after the call to m_inc(),
for all sizes of "n->m_len".