git: dc32441e3825 - stable/14 - ifnet: Defer detaching address family dependent data
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 27 Sep 2025 15:14:01 UTC
The branch stable/14 has been updated by zlei: URL: https://cgit.FreeBSD.org/src/commit/?id=dc32441e3825a90027b61259c3c77ef6e213728a commit dc32441e3825a90027b61259c3c77ef6e213728a Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2025-09-03 19:16:40 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2025-09-27 15:11:37 +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 (cherry picked from commit b5c46895fdddcdb7dd1994598925d6989ea7c8f2) --- 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 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);