ifnet: Defer detaching address family dependent data

While diagnosing PR 279653 and PR 285129, I observed that thread may
write to freed memory but the system does not crash. This hides the
real problem. A clear NULL pointer derefence is much better than writing
to freed memory.

PR:	279653
PR:	285129
Reviewed by:	glebius
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D49444

(cherry picked from commit b5c46895fdddcdb7dd1994598925d6989ea7c8f2)

3 files changed, 25 insertions, 5 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index f308edd24734..607bcdd2aa80 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1105,6 +1105,7 @@ if_detach_internal(struct ifnet *ifp, bool vmove)
 	struct ifaddr *ifa;
 	int i;
 	struct domain *dp;
+	void *if_afdata[AF_MAX];
 #ifdef VIMAGE
 	bool shutdown;
 
@@ -1228,15 +1229,30 @@ finish_vnet_shutdown:
 	IF_AFDATA_LOCK(ifp);
 	i = ifp->if_afdata_initialized;
 	ifp->if_afdata_initialized = 0;
+	if (i != 0) {
+		/*
+		 * Defer the dom_ifdetach call.
+		 */
+		_Static_assert(sizeof(if_afdata) == sizeof(ifp->if_afdata),
+		    "array size mismatch");
+		memcpy(if_afdata, ifp->if_afdata, sizeof(if_afdata));
+		memset(ifp->if_afdata, 0, sizeof(ifp->if_afdata));
+	}
 	IF_AFDATA_UNLOCK(ifp);
 	if (i == 0)
 		return;
+	/*
+	 * XXXZL: This net epoch wait is not necessary if we have done right.
+	 * But if we do not, at least we can make a guarantee that threads those
+	 * enter net epoch will see NULL address family dependent data,
+	 * e.g. if_afdata[AF_INET6]. A clear NULL pointer derefence is much
+	 * better than writing to freed memory.
+	 */
+	NET_EPOCH_WAIT();
 	SLIST_FOREACH(dp, &domains, dom_next) {
-		if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family]) {
-			(*dp->dom_ifdetach)(ifp,
-			    ifp->if_afdata[dp->dom_family]);
-			ifp->if_afdata[dp->dom_family] = NULL;
-		}
+		if (dp->dom_ifdetach != NULL &&
+		    if_afdata[dp->dom_family] != NULL)
+			(*dp->dom_ifdetach)(ifp, if_afdata[dp->dom_family]);
 	}
 }
 
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index 75ff9066875c..857f12a40ce7 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1872,6 +1872,8 @@ in_domifdetach(struct ifnet *ifp, void *aux)
 {
 	struct in_ifinfo *ii = (struct in_ifinfo *)aux;
 
+	MPASS(ifp->if_afdata[AF_INET] == NULL);
+
 	igmp_domifdetach(ifp);
 	lltable_free(ii->ii_llt);
 	free(ii, M_IFADDR);
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index b6ecf24a73cd..8d43ae18969a 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -2620,6 +2620,8 @@ in6_domifdetach(struct ifnet *ifp, void *aux)
 {
 	struct in6_ifextra *ext = (struct in6_ifextra *)aux;
 
+	MPASS(ifp->if_afdata[AF_INET6] == NULL);
+
 	mld_domifdetach(ifp);
 	scope6_ifdetach(ext->scope6_id);
 	nd6_ifdetach(ifp, ext->nd_ifinfo);