Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Move copyout() out of netlock within sysctl_ifnames()
To:
Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date:
Tue, 4 Nov 2025 12:16:26 +0300

Download raw body.

Thread
Bump reference counter and link desired interface descriptors into
temporary `if_tmplist' while holding shared netlock. This temporary
list is protected by `if_tmplist_lock' rwlock so release of the
netlock is fine. Do copyout() while holding `if_tmplist_lock' and
then tear down temporary list.

We follow this way in if_getgroupmembers() and related paths.

Index: sys/net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.388
diff -u -p -r1.388 rtsock.c
--- sys/net/rtsock.c	21 Aug 2025 08:49:21 -0000	1.388
+++ sys/net/rtsock.c	4 Nov 2025 12:26:25 -0000
@@ -2110,14 +2110,24 @@ sysctl_iflist(int af, struct walkarg *w)
 int
 sysctl_ifnames(struct walkarg *w)
 {
+	TAILQ_HEAD(, ifnet)     if_tmplist =
+	    TAILQ_HEAD_INITIALIZER(if_tmplist);
 	struct if_nameindex_msg ifn;
 	struct ifnet *ifp;
 	int error = 0;
 
+	rw_enter_write(&if_tmplist_lock);
+	NET_LOCK_SHARED();
 	/* XXX ignore tableid for now */
 	TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
 		if (w->w_arg && w->w_arg != ifp->if_index)
 			continue;
+		if_ref(ifp);
+		TAILQ_INSERT_TAIL(&if_tmplist, ifp, if_tmplist);
+	}
+	NET_UNLOCK_SHARED();
+
+	TAILQ_FOREACH (ifp, &if_tmplist, if_tmplist) {
 		w->w_needed += sizeof(ifn);
 		if (w->w_where && w->w_needed <= w->w_given) {
 
@@ -2127,12 +2137,18 @@ sysctl_ifnames(struct walkarg *w)
 			    sizeof(ifn.if_name));
 			error = copyout(&ifn, w->w_where, sizeof(ifn));
 			if (error)
-				return (error);
+				break;
 			w->w_where += sizeof(ifn);
 		}
 	}
 
-	return (0);
+	while ((ifp = TAILQ_FIRST(&if_tmplist))) {
+		TAILQ_REMOVE(&if_tmplist, ifp, if_tmplist);
+		if_put(ifp);
+	}
+	rw_exit_write(&if_tmplist_lock);
+
+	return (error);
 }
 
 int
@@ -2261,10 +2277,7 @@ sysctl_rtable(int *name, u_int namelen, 
 		break;
 
 	case NET_RT_IFNAMES:
-		NET_LOCK_SHARED();
-		error = sysctl_ifnames(&w);
-		NET_UNLOCK_SHARED();
-		break;
+		return (sysctl_ifnames(&w));
 	}
 	free(w.w_tmem, M_RTABLE, w.w_tmemsize);
 	if (where) {