From 81a2c9c441dc3231499d48ad9c0871b075ef6aa0 Mon Sep 17 00:00:00 2001
From: sam <sam@FreeBSD.org>
Date: Fri, 7 Nov 2003 01:47:52 +0000
Subject: Fix locking of the ip forwarding cache.  We were holding a reference
 to a routing table entry w/o bumping the reference count or locking against
 the entry being free'd.  This caused major havoc (for some reason it appeared
 most frequently for folks running natd).  Fix is to bump the reference count
 whenever we copy the route cache contents into a private copy so the entry
 cannot be reclaimed out from under us.  This is a short term fix as the
 forthcoming routing table changes will eliminate this cache entirely.

Supported by:	FreeBSD Foundation
---
 sys/netinet/ip_input.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

(limited to 'sys/netinet/ip_input.c')

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 09f3f41..26cc060 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -213,7 +213,9 @@ int fw_one_pass = 1;
 ip_dn_io_t *ip_dn_io_ptr;
 
 /*
- * One deep route cache for ip forwarding.
+ * One deep route cache for ip forwarding.  This is done
+ * very inefficiently.  We don't care as it's about to be
+ * replaced by something better.
  */
 static struct rtcache {
 	struct route	rc_ro;		/* most recently used route */
@@ -223,28 +225,37 @@ static struct rtcache {
 #define	RTCACHE_LOCK()		mtx_lock(&ip_fwdcache.rc_mtx)
 #define	RTCACHE_UNLOCK()	mtx_unlock(&ip_fwdcache.rc_mtx)
 #define	RTCACHE_LOCK_INIT() \
-	mtx_init(&ip_fwdcache.rc_mtx, "route cache", NULL, MTX_DEF);
+	mtx_init(&ip_fwdcache.rc_mtx, "route cache", NULL, MTX_DEF)
 #define	RTCACHE_LOCK_ASSERT()	mtx_assert(&ip_fwdcache.rc_mtx, MA_OWNED)
 
 /*
- * Get the current route cache contents.
+ * Get a copy of the current route cache contents.
  */
 #define	RTCACHE_GET(_ro) do {					\
+	struct rtentry *rt;					\
 	RTCACHE_LOCK();						\
 	*(_ro) = ip_fwdcache.rc_ro;				\
+	if ((rt = (_ro)->ro_rt) != NULL) {			\
+		RT_LOCK(rt);					\
+		rt->rt_refcnt++;				\
+		RT_UNLOCK(rt);					\
+	}							\
 	RTCACHE_UNLOCK();					\
 } while (0)
 
 /*
- * Update the cache contents.  We optimize this using
- * the routing table reference. XXX is this safe?
+ * Update the cache contents.
  */
 #define	RTCACHE_UPDATE(_ro) do {				\
-	if ((_ro)->ro_rt != ip_fwdcache.rc_ro.ro_rt) {		\
-		RTCACHE_LOCK();					\
+	struct rtentry *rt;					\
+	RTCACHE_LOCK();						\
+	rt = ip_fwdcache.rc_ro.ro_rt;				\
+	if ((_ro)->ro_rt != rt) {				\
 		ip_fwdcache.rc_ro = *(_ro);			\
-		RTCACHE_UNLOCK();				\
+		if (rt)						\
+			RTFREE(rt);				\
 	}							\
+	RTCACHE_UNLOCK();					\
 } while (0)
 
 /*
@@ -332,15 +343,14 @@ ip_init()
 void
 ip_forward_cacheinval(void)
 {
-	struct rtentry *rt = NULL;
+	struct rtentry *rt;
 
 	RTCACHE_LOCK();
 	rt = ip_fwdcache.rc_ro.ro_rt;
 	ip_fwdcache.rc_ro.ro_rt = 0;
-	RTCACHE_UNLOCK();
-
 	if (rt != NULL)
 		RTFREE(rt);
+	RTCACHE_UNLOCK();
 }
 
 /*
-- 
cgit v1.1