nd6_free assumes all routers are processed by kernel RA

[Roy Marples <roy%marples.name@localhost>]

This hasn't been the case for a long time if you're a dhcpcd user with a 
default config.
As such, it's possible for the default IPv6 router as set by dhcpcd 
could be erroneously gc'ed by nd6_free.

Attached is a patch which addresses this by no longer considering any 
kernel selected default router and just considering if it is a router or 
not.
This reduces the scope of the ND6_WLOCK taken as well as fixing an issue 
where we write to ln->ln_state without a lock being held.

I would like this commited fairly promptly as I want it pulled up to -9 
and -8 (probablly need some adjustments, I've not checked).

Roy

? sys/netinet6/nd6.xxx
Index: sys/netinet6/nd6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6.c,v
retrieving revision 1.259
diff -u -p -r1.259 nd6.c
--- sys/netinet6/nd6.c	22 Aug 2019 21:22:50 -0000	1.259
+++ sys/netinet6/nd6.c	22 Aug 2019 21:43:38 -0000
@@ -1189,7 +1189,6 @@ nd6_is_addr_neighbor(const struct sockad
 static void
 nd6_free(struct llentry *ln, int gc)
 {
-	struct nd_defrouter *dr;
 	struct ifnet *ifp;
 	struct in6_addr *in6;
 	struct sockaddr_in6 sin6;
@@ -1204,81 +1203,70 @@ nd6_free(struct llentry *ln, int gc)
 	 * even though it is not harmful, it was not really necessary.
 	 */
 
-	if (!ip6_forwarding) {
-		ND6_WLOCK();
-		dr = nd6_defrouter_lookup(in6, ifp);
-
-		if (dr != NULL && dr->expire &&
-		    ln->ln_state == ND6_LLINFO_STALE && gc) {
+	if (!ip6_forwarding && ln->ln_router) {
+		if (ln->ln_state == ND6_LLINFO_STALE && gc) {
 			/*
 			 * If the reason for the deletion is just garbage
-			 * collection, and the neighbor is an active default
+			 * collection, and the neighbor is an active
 			 * router, do not delete it.  Instead, reset the GC
 			 * timer using the router's lifetime.
-			 * Simply deleting the entry would affect default
+			 * Simply deleting the entry may affect default
 			 * router selection, which is not necessarily a good
 			 * thing, especially when we're using router preference
 			 * values.
 			 * XXX: the check for ln_state would be redundant,
 			 *      but we intentionally keep it just in case.
 			 */
-			if (dr->expire > time_uptime)
+			if (ln->ln_expire > time_uptime)
 				nd6_llinfo_settimer(ln,
-				    (dr->expire - time_uptime) * hz);
+				    (ln->ln_expire - time_uptime) * hz);
 			else
 				nd6_llinfo_settimer(ln, nd6_gctimer * hz);
-			ND6_UNLOCK();
 			LLE_WUNLOCK(ln);
 			return;
 		}
 
-		if (ln->ln_router || dr) {
-			/*
-			 * We need to unlock to avoid a LOR with nd6_rt_flush()
-			 * with the rnh and for the calls to
-			 * nd6_pfxlist_onlink_check() and nd6_defrouter_select() in the
-			 * block further down for calls into nd6_lookup().
-			 * We still hold a ref.
-			 */
-			LLE_WUNLOCK(ln);
-
-			/*
-			 * nd6_rt_flush must be called whether or not the neighbor
-			 * is in the Default Router List.
-			 * See a corresponding comment in nd6_na_input().
-			 */
-			nd6_rt_flush(in6, ifp);
-		}
+		ND6_WLOCK();
 
-		if (dr) {
-			/*
-			 * Unreachablity of a router might affect the default
-			 * router selection and on-link detection of advertised
-			 * prefixes.
-			 */
+		/*
+		 * We need to unlock to avoid a LOR with nd6_rt_flush()
+		 * with the rnh and for the calls to
+		 * nd6_pfxlist_onlink_check() and nd6_defrouter_select() in the
+		 * block further down for calls into nd6_lookup().
+		 * We still hold a ref.
+		 *
+		 * Temporarily fake the state to choose a new default
+		 * router and to perform on-link determination of
+		 * prefixes correctly.
+		 * Below the state will be set correctly,
+		 * or the entry itself will be deleted.
+		 */
+		ln->ln_state = ND6_LLINFO_INCOMPLETE;
+		LLE_WUNLOCK(ln);
 
-			/*
-			 * Temporarily fake the state to choose a new default
-			 * router and to perform on-link determination of
-			 * prefixes correctly.
-			 * Below the state will be set correctly,
-			 * or the entry itself will be deleted.
-			 */
-			ln->ln_state = ND6_LLINFO_INCOMPLETE;
+		/*
+		 * nd6_rt_flush must be called whether or not the neighbor
+		 * is in the Default Router List.
+		 * See a corresponding comment in nd6_na_input().
+		 */
+		nd6_rt_flush(in6, ifp);
 
-			/*
-			 * Since nd6_defrouter_select() does not affect the
-			 * on-link determination and MIP6 needs the check
-			 * before the default router selection, we perform
-			 * the check now.
-			 */
-			nd6_pfxlist_onlink_check();
+		/*
+		 * Unreachablity of a router might affect the default
+		 * router selection and on-link detection of advertised
+		 * prefixes.
+		 *
+		 * Since nd6_defrouter_select() does not affect the
+		 * on-link determination and MIP6 needs the check
+		 * before the default router selection, we perform
+		 * the check now.
+		 */
+		nd6_pfxlist_onlink_check();
 
-			/*
-			 * refresh default router list
-			 */
-			nd6_defrouter_select();
-		}
+		/*
+		 * refresh default router list
+		 */
+		nd6_defrouter_select();
 
 #ifdef __FreeBSD__
 		/*
@@ -1288,10 +1276,9 @@ nd6_free(struct llentry *ln, int gc)
 		if (ln->la_flags & LLE_REDIRECT)
 			nd6_free_redirect(ln);
 #endif
-		ND6_UNLOCK();
 
-		if (ln->ln_router || dr)
-			LLE_WLOCK(ln);
+		ND6_UNLOCK();
+		LLE_WLOCK(ln);
 	}
 
 	sockaddr_in6_init(&sin6, in6, 0, 0, 0);