From owner-freebsd-stable@FreeBSD.ORG Thu Apr 8 11:42:24 2004 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id ADB0316A4CE; Thu, 8 Apr 2004 11:42:24 -0700 (PDT) Received: from rwcrmhc13.comcast.net (rwcrmhc13.comcast.net [204.127.198.39]) by mx1.FreeBSD.org (Postfix) with ESMTP id 869C443D53; Thu, 8 Apr 2004 11:42:24 -0700 (PDT) (envelope-from julian@elischer.org) Received: from interjet.elischer.org ([24.7.73.28]) by comcast.net (rwcrmhc13) with ESMTP id <2004040818422301500a9gn3e>; Thu, 8 Apr 2004 18:42:24 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id LAA64035; Thu, 8 Apr 2004 11:42:23 -0700 (PDT) Date: Thu, 8 Apr 2004 11:42:21 -0700 (PDT) From: Julian Elischer To: Ruslan Ermilov In-Reply-To: <20040408100929.GD16290@ip.net.ua> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: stable@FreeBSD.org cc: Julian Elischer cc: Archie Cobbs Subject: Re: ng_bridge(4) has an easily exploitable memory leak X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Apr 2004 18:42:24 -0000 looks good but: + if (destLink == firstLink) { + /* + * If we've sent all the others, send the original + * on the first link we found. + */ + NG_SEND_DATA(error, destLink->hook, m, meta); + break; /* always done last - not really needed. */ + } else { + NG_SEND_DATA(error, destLink->hook, m2, meta2); + } couldn't this be avoided by previously doing: + if (linkNum == priv->numLinks) { + /* If we never saw a good link, leave. */ + if (firstLink == NULL) { + NG_FREE_DATA(m, meta); + return (0); + } + destLink = firstLink; ---> m2 = m; ---> meta2 = meta; ---> m=NULL; ---> meta=NULL; + } I leave it up to you to decide which you prefer, (but remember that NG_SEND_DATA is a macro and expads somewhat. specifically, to (sorry about linewrap): #define NG_SEND_DATA(error, hook, m, meta) \ do {\ item_p _item; \ if ((_item = ng_package_data((m), (meta)))) {\ NG_FWD_ITEM_HOOK(error, _item, hook); \ } else { \ (error) = ENOMEM; \ }\ (m) = NULL; \ (meta) = NULL; \ } while (0) where NG_FWD_ITEM_HOOK itself expands to: #define NG_FWD_ITEM_HOOK(error, item, hook) \ do { \ (error) = \ ng_address_hook(NULL, (item), (hook), 0); \ if (error == 0) { \ SAVE_LINE(item); \ (error) = ng_snd_item((item), 0); \ } \ (item) = NULL; \ } while (0) so only having one of those saves a bit of code. On Thu, 8 Apr 2004, Ruslan Ermilov wrote: > On Wed, Apr 07, 2004 at 12:28:38PM -0700, Julian Elischer wrote: > > On Wed, 7 Apr 2004, Ruslan Ermilov wrote: > > > > > On RELENG_4, ng_bridge(4) has an easily exploitable memory leak, > > > and may quickly run system out of mbufs. It's enough to just > > > have only one link connected to the bridge, e.g., the "upper" > > > hook of the ng_ether(4) with IP address assigned, and pinging > > > the broadcast IP address on the interface. The bug is more > > > real when constructing a bridge, or, like we experienced it, > > > by shutting down all except one bridge's link. The following > > > patch fixes it: > > > > [snipped] > > > > An alternate solution is to MFC most of ng_bridge.c,v 1.8. Julian? > > > > what does an MFC diff look like? > > (bridge is one of archies's nodes) > > > But it was _you_ who masked this bugfix (which would equally apply > to RELENG_4 as well) under the "SMP preparation" emblem. ;) > > Anyway, the patch for RELENG_4 is attached. I've only compile tested > it. > > > Cheers, > -- > Ruslan Ermilov > ru@FreeBSD.org > FreeBSD committer >