Date: Thu, 8 Apr 2004 11:42:21 -0700 (PDT) From: Julian Elischer <julian@elischer.org> To: Ruslan Ermilov <ru@FreeBSD.org> Cc: Archie Cobbs <archie@FreeBSD.org> Subject: Re: ng_bridge(4) has an easily exploitable memory leak Message-ID: <Pine.BSF.4.21.0404081053540.63085-100000@InterJet.elischer.org> In-Reply-To: <20040408100929.GD16290@ip.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0404081053540.63085-100000>