git: b5c46895fddd - main - ifnet: Defer detaching address family dependent data

The branch main has been updated by zlei:

URL: https://cgit.FreeBSD.org/src/commit/?id=b5c46895fdddcdb7dd1994598925d6989ea7c8f2

commit b5c46895fdddcdb7dd1994598925d6989ea7c8f2
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2025-09-03 19:16:40 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2025-09-03 19:16:40 +0000

    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
---
 sys/net/if.c       | 26 +++++++++++++++++++++-----
 sys/netinet/in.c   |  2 ++
 sys/netinet6/in6.c |  2 ++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 0fc30488f1e5..b6a798aa0fab 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1101,6 +1101,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;
 
@@ -1224,15 +1225,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 0e283a7d099d..75ff1f5f3d68 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1882,6 +1882,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 a9e6c4eaa51b..be6233d8e4f8 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -2618,6 +2618,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);