net: fix page fault in route_cache::lookup() when no route is found

Fixes cloudius-systems#475. When there is no route, in_rtalloc_ign() sets ro.ro_rt to NULL. This caused the following memcpy() to page fault: memcpy(ret, ro.ro_rt, sizeof(*ret)); This change makes lookup() exit early on failure and propagate the information back to the caller. Signed-off-by: Tomasz Grabiec <[email protected]> Signed-off-by: Pekka Enberg <[email protected]>
asias · Aug 28, 2014 · 78691c7 · 78691c7
1 parent d2c0a71
commit 78691c7
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 8 deletions.
diff --git a/bsd/sys/net/routecache.hh b/bsd/sys/net/routecache.hh
@@ -130,7 +130,9 @@ class route_cache {
 public:
     // Note that this returns a copy of a routing entry, *not* a pointer.
     // So the return value shouldn't be written to, nor, of course, be RTFREE'd.
-    static void lookup(struct bsd_sockaddr_in *dst, u_int fibnum, struct rtentry *ret) {
+    //
+    // Returns true when lookup succeeded, false otherwise
+    static bool lookup(struct bsd_sockaddr_in *dst, u_int fibnum, struct rtentry *ret) {
         // Only support fib 0, which is what we use anyway (see rt_numfibs in
         // route.cc).
         assert(fibnum == 0);
@@ -140,13 +142,17 @@ public:
             auto entry = c->search(dst->sin_addr.s_addr);
             if (entry) {
                 memcpy(ret, entry, sizeof(*ret));
-                return;
+                return true;
             }
         }
         // Not found in cache. Do the slow lookup
         struct route ro {};
         ro.ro_dst = *(struct bsd_sockaddr *)dst;
         in_rtalloc_ign(&ro, 0, fibnum);
+        if (!ro.ro_rt) {
+            RO_RTFREE(&ro);
+            return false;
+        }
         memcpy(ret, ro.ro_rt, sizeof(*ret));
         RO_RTFREE(&ro);
         ret->rt_refcnt = -1; // try to catch some monkey-business
@@ -161,6 +167,7 @@ public:
             cache.assign(new_cache);
             osv::rcu_dispose(old_cache);
         }
+        return true;
     }
 
     static void invalidate() {

diff --git a/bsd/sys/netinet/in_pcb.cc b/bsd/sys/netinet/in_pcb.cc
@@ -677,8 +677,11 @@ in_pcbladdr(struct inpcb *inp, struct in_addr *faddr, struct in_addr *laddr,
 	 */
 	if ((inp->inp_socket->so_options & SO_DONTROUTE) == 0)
 	{
-	    route_cache::lookup(sin, inp->inp_inc.inc_fibnum, &rte_one);
-	    sro.ro_rt = &rte_one;
+		if (route_cache::lookup(sin, inp->inp_inc.inc_fibnum, &rte_one)) {
+			sro.ro_rt = &rte_one;
+		} else {
+			sro.ro_rt = NULL;
+		}
 	}
 
 	/*

diff --git a/bsd/sys/netinet/ip_output.cc b/bsd/sys/netinet/ip_output.cc
@@ -257,8 +257,11 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro, int flags,
 			    ntohl(ip->ip_src.s_addr ^ ip->ip_dst.s_addr),
 			    inp ? inp->inp_inc.inc_fibnum : M_GETFIB(m));
 #else
-			route_cache::lookup(dst, inp ? inp->inp_inc.inc_fibnum : M_GETFIB(m), &rte_one);
-			ro->ro_rt = &rte_one;
+			if (route_cache::lookup(dst, inp ? inp->inp_inc.inc_fibnum : M_GETFIB(m), &rte_one)) {
+				ro->ro_rt = &rte_one;
+			} else {
+				ro->ro_rt = NULL;
+			}
 #endif
 			rte = ro->ro_rt;
 		}

diff --git a/bsd/sys/netinet/tcp_subr.cc b/bsd/sys/netinet/tcp_subr.cc
@@ -1687,8 +1687,11 @@ tcp_maxmtu(struct in_conninfo *inc, int *flags)
 		dst->sin_family = AF_INET;
 		dst->sin_len = sizeof(*dst);
 		dst->sin_addr = inc->inc_faddr;
-		route_cache::lookup(dst, inc->inc_fibnum, &rte_one);
-		sro.ro_rt = &rte_one;
+		if (route_cache::lookup(dst, inc->inc_fibnum, &rte_one)) {
+			sro.ro_rt = &rte_one;
+		} else {
+			sro.ro_rt = NULL;
+		}
 	}
 	if (sro.ro_rt != NULL) {
 		ifp = sro.ro_rt->rt_ifp;