Hi,
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:
%%%
Index: ng_bridge.c
==================================================================RCS file:
/home/ncvs/src/sys/netgraph/ng_bridge.c,v
retrieving revision 1.1.2.6
diff -u -p -r1.1.2.6 ng_bridge.c
--- ng_bridge.c 9 Jan 2004 08:58:06 -0000 1.1.2.6
+++ ng_bridge.c 7 Apr 2004 12:29:46 -0000
@@ -656,6 +656,11 @@ ng_bridge_rcvdata(hook_p hook, struct mb
link->stats.recvUnknown++;
}
+ /* If there's only one link, stop right here. */
+ if (priv->numLinks == 1) {
+ NG_FREE_DATA(m, meta);
+ return (0);
+ }
/* Distribute unknown, multicast, broadcast pkts to all other links */
for (linkNum = i = 0; i < priv->numLinks - 1; linkNum++) {
struct ng_bridge_link *const destLink = priv->links[linkNum];
%%%
An alternate solution is to MFC most of ng_bridge.c,v 1.8. Julian?
Cheers,
--
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url :
http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20040407/d45e4545/attachment.bin
On Wed, 7 Apr 2004, Ruslan Ermilov wrote:> Hi, > > 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: > > %%% > Index: ng_bridge.c > ==================================================================> RCS file: /home/ncvs/src/sys/netgraph/ng_bridge.c,v > retrieving revision 1.1.2.6 > diff -u -p -r1.1.2.6 ng_bridge.c > --- ng_bridge.c 9 Jan 2004 08:58:06 -0000 1.1.2.6 > +++ ng_bridge.c 7 Apr 2004 12:29:46 -0000 > @@ -656,6 +656,11 @@ ng_bridge_rcvdata(hook_p hook, struct mb > link->stats.recvUnknown++; > } > > + /* If there's only one link, stop right here. */ > + if (priv->numLinks == 1) { > + NG_FREE_DATA(m, meta); > + return (0); > + } > /* Distribute unknown, multicast, broadcast pkts to all other links */ > for (linkNum = i = 0; i < priv->numLinks - 1; linkNum++) { > struct ng_bridge_link *const destLink = priv->links[linkNum]; > %%% > > 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)> > > Cheers, > -- > Ruslan Ermilov > ru@FreeBSD.org > FreeBSD committer >
Ruslan, B IGNORE THE PREVIOUS EMAIL I was looking at how the macro expanded in -current, not in -stable.. [...]> > > 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. >Maybe I should listen to myself and fix -current to follow my own advice!