Several fixes to rt_setgate(), that fix problems with route changing:

- Rearrange code so that in a case of failure the affected
  route is not changed. Otherwise, a bogus rtentry will be
  left and later rt_check() can recurse on its lock. [1]
- Remove comment about protocol cloning.
- Fix two places where rtentry mutex was recursed on, because
  accessed via two different pointers, that were actually pointing
  to the same rtentry in some cases. [1]
- Return EADDRINUSE instead of bogus EDQUOT, in case when gateway
  uses the same route. [2]

Reported & tested by:	ps, Andrej Zverev <az inec.ru> [1]
PR:			kern/64090 [2]

1 files changed, 30 insertions, 35 deletions

diff --git a/sys/net/route.c b/sys/net/route.c
index 447d56d..6fc33f2 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1016,6 +1016,36 @@ rt_setgate(struct rtentry *rt, struct sockaddr *dst, struct sockaddr *gate)
 	}
 
 	/*
+	 * Cloning loop avoidance in case of bad configuration.
+	 */
+	if (rt->rt_flags & RTF_GATEWAY) {
+		struct rtentry *gwrt;
+
+		RT_UNLOCK(rt);		/* XXX workaround LOR */
+		gwrt = rtalloc1(gate, 1, 0);
+		if (gwrt == rt) {
+			RT_LOCK_ASSERT(rt);
+			RT_REMREF(rt);
+			return (EADDRINUSE); /* failure */
+		}
+		RT_LOCK(rt);
+		/*
+		 * If there is already a gwroute, then drop it. If we
+		 * are asked to replace route with itself, then do
+		 * not leak its refcounter.
+		 */
+		if (rt->rt_gwroute != NULL) {
+			if (rt->rt_gwroute == gwrt) {
+				RT_REMREF(rt->rt_gwroute);
+			} else
+				RTFREE(rt->rt_gwroute);
+		}
+
+		if ((rt->rt_gwroute = gwrt) != NULL)
+			RT_UNLOCK(rt->rt_gwroute);
+	}
+
+	/*
 	 * Prepare to store the gateway in rt->rt_gateway.
 	 * Both dst and gateway are stored one after the other in the same
 	 * malloc'd chunk. If we have room, we can reuse the old buffer,
@@ -1047,41 +1077,6 @@ rt_setgate(struct rtentry *rt, struct sockaddr *dst, struct sockaddr *gate)
 	bcopy(gate, rt->rt_gateway, glen);
 
 	/*
-	 * If there is already a gwroute, it's now almost definitly wrong
-	 * so drop it.
-	 */
-	if (rt->rt_gwroute != NULL) {
-		RTFREE(rt->rt_gwroute);
-		rt->rt_gwroute = NULL;
-	}
-	/*
-	 * Cloning loop avoidance:
-	 * In the presence of protocol-cloning and bad configuration,
-	 * it is possible to get stuck in bottomless mutual recursion
-	 * (rtrequest rt_setgate rtalloc1).  We avoid this by not allowing
-	 * protocol-cloning to operate for gateways (which is probably the
-	 * correct choice anyway), and avoid the resulting reference loops
-	 * by disallowing any route to run through itself as a gateway.
-	 * This is obviously mandatory when we get rt->rt_output().
-	 * XXX: After removal of PRCLONING this is probably not needed anymore.
-	 */
-	if (rt->rt_flags & RTF_GATEWAY) {
-		struct rtentry *gwrt;
-
-		RT_UNLOCK(rt);		/* XXX workaround LOR */
-		gwrt = rtalloc1(gate, 1, 0);
-		RT_LOCK(rt);
-		rt->rt_gwroute = gwrt;
-		if (rt->rt_gwroute == rt) {
-			RTFREE_LOCKED(rt->rt_gwroute);
-			rt->rt_gwroute = NULL;
-			return EDQUOT; /* failure */
-		}
-		if (rt->rt_gwroute != NULL)
-			RT_UNLOCK(rt->rt_gwroute);
-	}
-
-	/*
 	 * This isn't going to do anything useful for host routes, so
 	 * don't bother.  Also make sure we have a reasonable mask
 	 * (we don't yet have one during adds).