aboutsummaryrefslogtreecommitdiffstats
path: root/net/ipv4
diff options
context:
space:
mode:
authorNeil Horman <nhorman@tuxdriver.com>2009-06-20 04:15:16 -0400
committerDavid S. Miller <davem@davemloft.net>2009-06-20 04:15:16 -0400
commit73e42897e8e5619eacb787d2ce69be12f47cfc21 (patch)
tree02ce26705cbca451b069205069afb0956deb9e04 /net/ipv4
parent952e57ba3769d6fc6139b8a99c32ea2bb63f23e9 (diff)
ipv4: fix NULL pointer + success return in route lookup path
Don't drop route if we're not caching I recently got a report of an oops on a route lookup. Maxime was testing what would happen if route caching was turned off (doing so by setting making rt_caching always return 0), and found that it triggered an oops. I looked at it and found that the problem stemmed from the fact that the route lookup routines were returning success from their lookup paths (which is good), but never set the **rp pointer to anything (which is bad). This happens because in rt_intern_hash, if rt_caching returns false, we call rt_drop and return 0. This almost emulates slient success. What we should be doing is assigning *rp = rt and _not_ dropping the route. This way, during slow path lookups, when we create a new route cache entry, we don't immediately discard it, rather we just don't add it into the cache hash table, but we let this one lookup use it for the purpose of this route request. Maxime has tested and reports it prevents the oops. There is still a subsequent routing issue that I'm looking into further, but I'm confident that, even if its related to this same path, this patch makes sense to take. Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/ipv4')
-rw-r--r--net/ipv4/route.c14
1 files changed, 12 insertions, 2 deletions
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index cd76b3cb7092..65b3a8b11a6c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1085,8 +1085,16 @@ restart:
1085 now = jiffies; 1085 now = jiffies;
1086 1086
1087 if (!rt_caching(dev_net(rt->u.dst.dev))) { 1087 if (!rt_caching(dev_net(rt->u.dst.dev))) {
1088 rt_drop(rt); 1088 /*
1089 return 0; 1089 * If we're not caching, just tell the caller we
1090 * were successful and don't touch the route. The
1091 * caller hold the sole reference to the cache entry, and
1092 * it will be released when the caller is done with it.
1093 * If we drop it here, the callers have no way to resolve routes
1094 * when we're not caching. Instead, just point *rp at rt, so
1095 * the caller gets a single use out of the route
1096 */
1097 goto report_and_exit;
1090 } 1098 }
1091 1099
1092 rthp = &rt_hash_table[hash].chain; 1100 rthp = &rt_hash_table[hash].chain;
@@ -1217,6 +1225,8 @@ restart:
1217 rcu_assign_pointer(rt_hash_table[hash].chain, rt); 1225 rcu_assign_pointer(rt_hash_table[hash].chain, rt);
1218 1226
1219 spin_unlock_bh(rt_hash_lock_addr(hash)); 1227 spin_unlock_bh(rt_hash_lock_addr(hash));
1228
1229report_and_exit:
1220 if (rp) 1230 if (rp)
1221 *rp = rt; 1231 *rp = rt;
1222 else 1232 else