Date: Sat, 1 Jun 2002 12:57:07 +0300 From: Ruslan Ermilov <ru@FreeBSD.ORG> To: Andre Oppermann <oppermann@pipeline.ch> Cc: freebsd-net@FreeBSD.ORG, silby@silby.com Subject: Re: Bug in net/route.c function rtredirect() Message-ID: <20020601095707.GC50435@sunbay.com> In-Reply-To: <3CF81BC5.EAF0ACC@pipeline.ch> References: <3CF81BC5.EAF0ACC@pipeline.ch>
next in thread | previous in thread | raw e-mail | index | archive | help
--eHhjakXzOLJAF9wJ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Jun 01, 2002 at 02:56:37AM +0200, Andre Oppermann wrote:
> Hi all,
>=20
> I've found another bug in net/route.c in the function rtredirect().
>=20
> When learning a new gateway from a ICMP redirect icmp_input calls
> rtredirect() in net/route.c. rtredirect is doing some sanity checks
> and then creates, if it did not find an existing host route, a new
> host route tagged with the RTF_DYNAMIC flag. If there was an existing
> host route it'll simply adjust the gateway and set the RTF_MODIFIED
> flag.
>=20
OK.
> If no pre-existing host route was found either the default route or
> an network route is being returned by the route table lookup in the
> beginning. Neither of these should be modified. It should simply add
> the new host route.
>=20
This route lookup also increments rt_refcnt.
> The bug is that the jump to "create:" (host route) tries to rtfree()
> the found default route or network route. If the refcount on those
> routes is one it frees it, otherwise it decrements it by one. This is
> wrong. There is no reason to decrement the refcount or to free the
> routes. The bug is even dangerous when you happen to have many
> redirects, one too much and you'll loose your default route. The
> refcount drifts away one decrement more with every valid redirecet
> received.
>=20
There _is_ the reason to decrement the refcount, because nothing is
going to point to it after we modify it, and if we don't decrement
it, this will create an "unremovable" route.
> The solution is to remove the rtfree() in rtredirect. Diff against
> -STABLE is attached. Should apply to -CURRENT with some fuzz.
>=20
No, please try the below patch instead.
> The bug has been introduced by ru in commit 1.67 "Pull post-4.4BSD
> change from BSD/OS 4.2". This bug is also present in NetBSD and was
> introduced there before here (1.67 commit is a copy from NetBSD).
>=20
Why you're not mailing me then? The bug was introduced first in
BSD/OS, FWIW.
> I found this by observing a problem on a machine on my network where
> I have two routers. One is the default router for the machine. Some-
> times it'll get a redirect to use the second router and then the
> inconsistencies begun. I only really noticed this problem by using
> and watching the numbers of the tcp statistics code I posted yesterday
> (plus route -n monior). This is side-work for an overhaul of the
> kernel routing table.
>=20
I will see if I can reproduce this, on Monday.
> While being in this area I noticed that host routes created by a
> redirect never time out and never get removed in a regular fashion.
> So if you get many of them they clutter the whole routing table. This
> can become dangerous if you get a lot tcp sessions and your default
> router is sub-optimal and redirects you all the time to the "real
> default" router on your network. There can be a redirected route for
> every host on the Internet. Bad if a new code-red or nimda wave comes
> by. NetBSD has implemented a purger for the redirect host routes.
> I'll have a look at it and provide a patch for that for FreeBSD next
> week.
>=20
Nice catch.
Please try this patch, it's believed to fix both problems. It is
completely untested:
%%%
Index: route.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/net/route.c,v
retrieving revision 1.69
diff -u -p -r1.69 route.c
--- route.c 19 Mar 2002 21:54:18 -0000 1.69
+++ route.c 1 Jun 2002 09:56:11 -0000
@@ -345,7 +345,7 @@ rtredirect(dst, gateway, netmask, flags,
*/
create:
if (rt)
- rtfree(rt);
+ rt->rt_refcnt--;
flags |=3D RTF_GATEWAY | RTF_DYNAMIC;
bzero((caddr_t)&info, sizeof(info));
info.rti_info[RTAX_DST] =3D dst;
@@ -355,8 +355,10 @@ rtredirect(dst, gateway, netmask, flags,
info.rti_flags =3D flags;
rt =3D NULL;
error =3D rtrequest1(RTM_ADD, &info, &rt);
- if (rt !=3D NULL)
+ if (rt !=3D NULL) {
+ rt->rt_refcnt--;
flags =3D rt->rt_flags;
+ }
stat =3D &rtstat.rts_dynamic;
} else {
/*
%%%
Cheers,
--=20
Ruslan Ermilov Sysadmin and DBA,
ru@sunbay.com Sunbay Software AG,
ru@FreeBSD.org FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
--eHhjakXzOLJAF9wJ
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (FreeBSD)
iD8DBQE8+JpzUkv4P6juNwoRAvxJAJ9jhqZ2QsDSOUyYMvlaScKy84YSLgCeJWIy
ZXOHZrt6xuNxpq4TGEdGkeY=
=Ngt3
-----END PGP SIGNATURE-----
--eHhjakXzOLJAF9wJ--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020601095707.GC50435>