Date: Thu, 8 Apr 2004 13:09:29 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: Julian Elischer <julian@elischer.org>, Archie Cobbs <archie@FreeBSD.org> Cc: Julian Elischer <julian@FreeBSD.org> Subject: Re: ng_bridge(4) has an easily exploitable memory leak Message-ID: <20040408100929.GD16290@ip.net.ua> In-Reply-To: <Pine.BSF.4.21.0404071227420.34985-100000@InterJet.elischer.org> References: <20040407191003.GA1136@ip.net.ua> <Pine.BSF.4.21.0404071227420.34985-100000@InterJet.elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--jkO+KyKz7TfD21mV
Content-Type: multipart/mixed; boundary="gMR3gsNFwZpnI/Ts"
Content-Disposition: inline
--gMR3gsNFwZpnI/Ts
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Wed, Apr 07, 2004 at 12:28:38PM -0700, Julian Elischer wrote:
> On Wed, 7 Apr 2004, Ruslan Ermilov wrote:
>=20
> > 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:
> >=20
[snipped]
> > An alternate solution is to MFC most of ng_bridge.c,v 1.8. Julian?
>=20
> what does an MFC diff look like?
> (bridge is one of archies's nodes)
>=20
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,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer
--gMR3gsNFwZpnI/Ts
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=p
Content-Transfer-Encoding: quoted-printable
Index: ng_bridge.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 8 Apr 2004 09:53:43 -0000
@@ -518,7 +518,8 @@ ng_bridge_rcvdata(hook_p hook, struct mb
struct ng_bridge_link *link;
struct ether_header *eh;
int error =3D 0, linkNum;
- int i, manycast;
+ int manycast;
+ struct ng_bridge_link *firstLink;
=20
/* Get link number */
linkNum =3D LINK_NUM(hook);
@@ -657,20 +658,43 @@ ng_bridge_rcvdata(hook_p hook, struct mb
}
=20
/* Distribute unknown, multicast, broadcast pkts to all other links */
- for (linkNum =3D i =3D 0; i < priv->numLinks - 1; linkNum++) {
- struct ng_bridge_link *const destLink =3D priv->links[linkNum];
+ firstLink =3D NULL;
+ for (linkNum =3D 0; linkNum <=3D priv->numLinks; linkNum++) {
+ struct ng_bridge_link *destLink;
meta_p meta2 =3D NULL;
- struct mbuf *m2;
+ struct mbuf *m2 =3D NULL;
=20
- /* Skip incoming link and disconnected links */
- if (destLink =3D=3D NULL || destLink =3D=3D link)
- continue;
-
- /* Copy mbuf and meta info */
- if (++i =3D=3D priv->numLinks - 1) { /* last link */
- m2 =3D m;
- meta2 =3D meta;
- } else {
+ /*
+ * If we have checked all the links then now
+ * send the original on its reserved link
+ */
+ if (linkNum =3D=3D priv->numLinks) {
+ /* If we never saw a good link, leave. */
+ if (firstLink =3D=3D NULL) {
+ NG_FREE_DATA(m, meta);
+ return (0);
+ }=09
+ destLink =3D firstLink;
+ } else {
+ destLink =3D priv->links[linkNum];
+ /* Skip incoming link and disconnected links */
+ if (destLink =3D=3D NULL || destLink =3D=3D link) {
+ continue;
+ }
+ if (firstLink =3D=3D NULL) {
+ /*
+ * This is the first usable link we have found.
+ * Reserve it for the originals.
+ * If we never find another we save a copy.
+ */
+ firstLink =3D destLink;
+ continue;
+ }
+
+ /*
+ * It's usable link but not the reserved (first) one.
+ * Copy mbuf and meta info for sending.
+ */
m2 =3D m_dup(m, M_NOWAIT); /* XXX m_copypacket() */
if (m2 =3D=3D NULL) {
link->stats.memoryFailures++;
@@ -701,7 +725,16 @@ ng_bridge_rcvdata(hook_p hook, struct mb
}
=20
/* Send packet */
- NG_SEND_DATA(error, destLink->hook, m2, meta2);
+ if (destLink =3D=3D firstLink) {=20
+ /*
+ * 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);
+ }
}
return (error);
}
--gMR3gsNFwZpnI/Ts--
--jkO+KyKz7TfD21mV
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)
iD8DBQFAdSTZUkv4P6juNwoRAp/cAJ4pDK1arhdvuPRodwJgPhMhv07oRQCdH9fX
xet0k5yoCDF04g6dHqTzWsQ=
=Mo22
-----END PGP SIGNATURE-----
--jkO+KyKz7TfD21mV--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040408100929.GD16290>