Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 15 Jun 2026 18:01:53 +0000
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 8da837bba1de - stable/15 - net: Fix handling of unmapped user pages in if_getgroup()
Message-ID:  <6a303e11.20254.518059cd@gitrepo.freebsd.org>
index | next in thread | raw e-mail 

The branch stable/15 has been updated by markj:

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

commit 8da837bba1de7c9fb754dbed87378cb6815a89fa
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-06-01 16:44:15 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-06-15 16:00:04 +0000

    net: Fix handling of unmapped user pages in if_getgroup()
    
    We cannot call copyout() while in a net epoch section, unless the user
    memory is wired.  Use the global ifnet lock to synchronize the accesses
    instead.
    
    Reported by:    emaste
    Reviewed by:    zlei
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D57154
    
    (cherry picked from commit 68004e56fdc22c11b4ec680e83309b4ea2bfe13a)
---
 sys/net/if.c         | 56 +++++++++++++++++++++-------------------------------
 sys/net/if_private.h |  2 +-
 2 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 0252ea742f66..ed22bbd8f8da 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1484,11 +1484,8 @@ if_addgroup(struct ifnet *ifp, const char *groupname)
 	ifgl->ifgl_group = ifg;
 	ifgm->ifgm_ifp = ifp;
 
-	IF_ADDR_WLOCK(ifp);
 	CK_STAILQ_INSERT_TAIL(&ifg->ifg_members, ifgm, ifgm_next);
 	CK_STAILQ_INSERT_TAIL(&ifp->if_groups, ifgl, ifgl_next);
-	IF_ADDR_WUNLOCK(ifp);
-
 	IFNET_WUNLOCK();
 
 	if (new)
@@ -1511,9 +1508,7 @@ _if_delgroup_locked(struct ifnet *ifp, struct ifg_list *ifgl,
 
 	IFNET_WLOCK_ASSERT();
 
-	IF_ADDR_WLOCK(ifp);
 	CK_STAILQ_REMOVE(&ifp->if_groups, ifgl, ifg_list, ifgl_next);
-	IF_ADDR_WUNLOCK(ifp);
 
 	CK_STAILQ_FOREACH(ifgm, &ifgl->ifgl_group->ifg_members, ifgm_next) {
 		if (ifgm->ifgm_ifp == ifp) {
@@ -1588,34 +1583,35 @@ if_delgroups(struct ifnet *ifp)
 static int
 if_getgroup(struct ifgroupreq *ifgr, struct ifnet *ifp)
 {
-	int			 len, error;
-	struct ifg_list		*ifgl;
-	struct ifg_req		 ifgrq, *ifgp;
-
-	NET_EPOCH_ASSERT();
+	struct ifg_list *ifgl;
+	struct ifg_req ifgrq, *ifgp;
+	int len, error;
 
+	IFNET_RLOCK();
 	if (ifgr->ifgr_len == 0) {
 		CK_STAILQ_FOREACH(ifgl, &ifp->if_groups, ifgl_next)
 			ifgr->ifgr_len += sizeof(struct ifg_req);
-		return (0);
-	}
-
-	len = ifgr->ifgr_len;
-	ifgp = ifgr->ifgr_groups;
-	/* XXX: wire */
-	CK_STAILQ_FOREACH(ifgl, &ifp->if_groups, ifgl_next) {
-		if (len < sizeof(ifgrq))
-			return (EINVAL);
-		bzero(&ifgrq, sizeof ifgrq);
-		strlcpy(ifgrq.ifgrq_group, ifgl->ifgl_group->ifg_group,
-		    sizeof(ifgrq.ifgrq_group));
-		if ((error = copyout(&ifgrq, ifgp, sizeof(struct ifg_req))))
-			return (error);
-		len -= sizeof(ifgrq);
-		ifgp++;
+		error = 0;
+	} else {
+		len = ifgr->ifgr_len;
+		ifgp = ifgr->ifgr_groups;
+		CK_STAILQ_FOREACH(ifgl, &ifp->if_groups, ifgl_next) {
+			if (len < sizeof(ifgrq)) {
+				error = EINVAL;
+				break;
+			}
+			bzero(&ifgrq, sizeof ifgrq);
+			strlcpy(ifgrq.ifgrq_group, ifgl->ifgl_group->ifg_group,
+			    sizeof(ifgrq.ifgrq_group));
+			if ((error = copyout(&ifgrq, ifgp, sizeof(struct ifg_req))))
+				break;
+			len -= sizeof(ifgrq);
+			ifgp++;
+		}
 	}
+	IFNET_RUNLOCK();
 
-	return (0);
+	return (error);
 }
 
 /*
@@ -2867,14 +2863,8 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 		break;
 	}
 	case SIOCGIFGROUP:
-	{
-		struct epoch_tracker et;
-
-		NET_EPOCH_ENTER(et);
 		error = if_getgroup((struct ifgroupreq *)data, ifp);
-		NET_EPOCH_EXIT(et);
 		break;
-	}
 
 	case SIOCDIFGROUP:
 	{
diff --git a/sys/net/if_private.h b/sys/net/if_private.h
index 3da529e6b22e..a5cc09127f6f 100644
--- a/sys/net/if_private.h
+++ b/sys/net/if_private.h
@@ -41,7 +41,7 @@ struct ifnet {
 	CK_STAILQ_ENTRY(ifnet) if_link; 	/* all struct ifnets are chained (CK_) */
 	LIST_ENTRY(ifnet) if_clones;	/* interfaces of a cloner */
 	CK_STAILQ_HEAD(, ifg_list) if_groups; /* linked list of groups per if (CK_) */
-					/* protected by if_addr_lock */
+					/* protected by ifnet_sxlock */
 	u_char	if_alloctype;		/* if_type at time of allocation */
 	uint8_t	if_numa_domain;		/* NUMA domain of device */
 	/* Driver and protocol specific information that remains stable. */
home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a303e11.20254.518059cd>