From owner-freebsd-net@FreeBSD.ORG Wed Apr 30 23:27:00 2003 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DA70837B401 for ; Wed, 30 Apr 2003 23:27:00 -0700 (PDT) Received: from angelica.unixdaemons.com (angelica.unixdaemons.com [209.148.64.135]) by mx1.FreeBSD.org (Postfix) with ESMTP id EAB2E43FB1 for ; Wed, 30 Apr 2003 23:26:59 -0700 (PDT) (envelope-from bmilekic@unixdaemons.com) Received: from angelica.unixdaemons.com (bmilekic@localhost.unixdaemons.com [127.0.0.1])h416Qwww028881; Thu, 1 May 2003 02:26:58 -0400 (EDT) Received: (from bmilekic@localhost) by angelica.unixdaemons.com (8.12.9/8.12.1/Submit) id h416Qwr9028880; Thu, 1 May 2003 02:26:58 -0400 (EDT) (envelope-from bmilekic@unixdaemons.com) X-Authentication-Warning: angelica.unixdaemons.com: bmilekic set sender to bmilekic@unixdaemons.com using -f Date: Thu, 1 May 2003 02:26:58 -0400 From: Bosko Milekic To: Mike Silbersack Message-ID: <20030501062658.GA26458@unixdaemons.com> References: <20030430142532.F3741@odysseus.silby.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030430142532.F3741@odysseus.silby.com> User-Agent: Mutt/1.4.1i cc: freebsd-net@freebsd.org Subject: Re: Review needed: Mbuf double-free detection patch X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 May 2003 06:27:01 -0000 On Wed, Apr 30, 2003 at 02:35:23PM -0500, Mike Silbersack wrote: > > I'd be interested in comments on the attached patch from anyone who's been > doing work with network drivers & such. All it does is add a M_FREELIST > flag which is set whenever a mbuf is freed. If m_free or m_freem find > this flag to be set, they will panic, as this is a clear sign that the > mbuf was freed twice. (All flags are cleared whenever a mbuf is > taken off the freelist, so false M_FREELIST hits shouldn't occur.) > > The system isn't perfect, as it won't catch mbufs which are reallocated > before their second free occurs. However, it does seem to do a good job > in catching simple double-free errors, which previously caused corruption > that lead to panics in codepaths totally unrelated to the original > double-free. (One of my double-free tests without this code managed to > cause a mutex-related panic, somehow!) > > I could probably make this code test for use-after-free by checksumming > the entire mbuf when M_FREELIST is set and verifying that the checksum has > not changed when the mbuf is reallocated, but I think this code is useful > enough as it is. > > Comments? > > Thanks, This sounds like a good idea but there is a potential bogon if you enable it by default. There is a certain producer/consumer relationship with mbuf consumption in some cases and in which case you would have one thread allocating a bunch of mbufs, writing to them, etc., and another thread reclaiming the data, but not writing to it, and then freeing the mbufs. If I understand it correctly, your patch introduces a write-to-mbuf-data on free, which may force unnecessary slot invalidations... this may sound a little "cooked up," but there is certainly an effort to not write to the object being freed during free so as to not force unnecessary invalidations in the producer/consumer cases such as the one described. -- Bosko Milekic bmilekic@unixdaemons.com bmilekic@FreeBSD.org