Date: Wed, 3 Sep 2025 19:17:25 GMT From: Zhenlei Huang <zlei@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: b5c46895fddd - main - ifnet: Defer detaching address family dependent data Message-ID: <202509031917.583JHP2x037832@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202509031917.583JHP2x037832>