git: 5c4021ca0abe - stable/15 - ifnet: if_detach(): Fix races with vmove operations

The branch stable/15 has been updated by zlei:

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

commit 5c4021ca0abe4e17200f5faa2fd71014ef0a5f09
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2026-04-25 19:56:07 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2026-04-30 09:52:02 +0000

    ifnet: if_detach(): Fix races with vmove operations
    
    The rationality is that the driver private data holds a strong reference
    to the interface, and the detach operation shall never fail. Given the
    vmove operation, if_vmove_loan(), if_vmove_reclaim() or vnet_if_return()
    is not atomic and spans multiple steps, acquire ifnet_detach_sxlock only
    for if_detach_internal() and if_vmove() is not sufficient. It is possible
    that the thread running if_detach() sees stale vnet, or the vmoving is
    in progress, then if_unlink_ifnet() will fail.
    
    Fix that by extending coverage of ifnet_detach_sxlock a bit to also
    cover if_unlink_ifnet(), so that the entire detach and vmove operation
    is serialized.
    
    Given it is an error when the if_unlink_ifnet() fails, and if_detach()
    is a public KPI, prefer panic() over assertion on failure, to indicate
    explicitly that bad thing happens. That shall also prevent potential
    corrupted status of the interface, which is a bit hard to diagnose.
    
    PR:             292993
    Reviewed by:    glebius
    MFC after:      5 days
    Differential Revision:  https://reviews.freebsd.org/D56374
    
    (cherry picked from commit ba7f47d47dc1a177e4d8f115f791ec25f3da0eab)
---
 sys/net/if.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 223082b675e4..0252ea742f66 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -457,6 +457,7 @@ if_unlink_ifnet(struct ifnet *ifp, bool vmove)
 	struct ifnet *iter;
 	int found = 0;
 
+	sx_assert(&ifnet_detach_sxlock, SX_XLOCKED);
 	IFNET_WLOCK();
 	CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
 		if (iter == ifp) {
@@ -1083,14 +1084,23 @@ if_detach(struct ifnet *ifp)
 {
 	bool found;
 
+	/*
+	 * The driver private data holds a strong reference to the ifnet, and
+	 * it is actually the "owner", hence this routine shall never fail.
+	 *
+	 * Ideally we can loop retrying when we lose race with other threads
+	 * those run if_unlink_ifnet(). For simplicity, use ifnet_detach_sxlock
+	 * to serialize all the detach / vmove operations.
+	 */
+	sx_xlock(&ifnet_detach_sxlock);
 	CURVNET_SET_QUIET(ifp->if_vnet);
 	found = if_unlink_ifnet(ifp, false);
-	if (found) {
-		sx_xlock(&ifnet_detach_sxlock);
-		if_detach_internal(ifp, false);
-		sx_xunlock(&ifnet_detach_sxlock);
-	}
+	if (! found)
+		panic("%s: interface is not on the active list",
+		    ifp->if_xname);
+	if_detach_internal(ifp, false);
 	CURVNET_RESTORE();
+	sx_xunlock(&ifnet_detach_sxlock);
 }
 
 /*
@@ -1399,13 +1409,14 @@ if_vmove_reclaim(struct thread *td, char *ifname, int jid)
 	}
 
 	/* Get interface back from child jail/vnet. */
+	sx_xlock(&ifnet_detach_sxlock);
 	found = if_unlink_ifnet(ifp, true);
 	if (! found) {
+		sx_xunlock(&ifnet_detach_sxlock);
 		CURVNET_RESTORE();
 		prison_free(pr);
 		return (ENODEV);
 	}
-	sx_xlock(&ifnet_detach_sxlock);
 	if_vmove(ifp, vnet_dst);
 	sx_xunlock(&ifnet_detach_sxlock);
 	CURVNET_RESTORE();