From: bluhm <bluhm@openbsd.org>
Date: Thu, 14 Jul 2022 13:46:24 +0000 (+0000)
Subject: Protect all writers to ifm_cur with a mutex.  ifmedia_match() does
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=566456819eeab3c855dd1dba674e689c3318cb0b;p=openbsd

Protect all writers to ifm_cur with a mutex.  ifmedia_match() does
not return any pointers without lock anymore.
OK mvs@ mbuhl@
---

diff --git a/sys/dev/ic/if_wi.c b/sys/dev/ic/if_wi.c
index 2b1547d77b2..dc6fbc39fa8 100644
--- a/sys/dev/ic/if_wi.c
+++ b/sys/dev/ic/if_wi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_wi.c,v 1.176 2022/01/09 05:42:38 jsg Exp $	*/
+/*	$OpenBSD: if_wi.c,v 1.177 2022/07/14 13:46:24 bluhm Exp $	*/
 
 /*
  * Copyright (c) 1997, 1998, 1999
@@ -2707,7 +2707,7 @@ wi_sync_media(struct wi_softc *sc, int ptype, int txrate)
 	}
 	media = IFM_MAKEWORD(IFM_TYPE(media), subtype, options,
 	IFM_INST(media));
-	if (ifmedia_match(&sc->sc_media, media, sc->sc_media.ifm_mask) == NULL)
+	if (!ifmedia_match(&sc->sc_media, media, sc->sc_media.ifm_mask))
 		return (EINVAL);
 	ifmedia_set(&sc->sc_media, media);
 	sc->wi_ptype = ptype;
diff --git a/sys/net/if_media.c b/sys/net/if_media.c
index 07cf22ce20d..15669186417 100644
--- a/sys/net/if_media.c
+++ b/sys/net/if_media.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_media.c,v 1.35 2022/07/12 22:27:38 bluhm Exp $	*/
+/*	$OpenBSD: if_media.c,v 1.36 2022/07/14 13:46:25 bluhm Exp $	*/
 /*	$NetBSD: if_media.c,v 1.10 2000/03/13 23:52:39 soren Exp $	*/
 
 /*-
@@ -105,6 +105,8 @@ static	void ifmedia_printword(uint64_t);
 
 struct mutex ifmedia_mtx = MUTEX_INITIALIZER(IPL_NET);
 
+struct	ifmedia_entry *ifmedia_get(struct ifmedia *, uint64_t, uint64_t);
+
 /*
  * Initialize if_media struct for a specific interface instance.
  */
@@ -181,7 +183,8 @@ ifmedia_set(struct ifmedia *ifm, uint64_t target)
 {
 	struct ifmedia_entry *match;
 
-	match = ifmedia_match(ifm, target, ifm->ifm_mask);
+	mtx_enter(&ifmedia_mtx);
+	match = ifmedia_get(ifm, target, ifm->ifm_mask);
 
 	/*
 	 * If we didn't find the requested media, then we try to fall
@@ -201,15 +204,20 @@ ifmedia_set(struct ifmedia *ifm, uint64_t target)
 		printf("%s: no match for 0x%llx/0x%llx\n", __func__,
 		    target, ~ifm->ifm_mask);
 		target = (target & IFM_NMASK) | IFM_NONE;
-		match = ifmedia_match(ifm, target, ifm->ifm_mask);
+		match = ifmedia_get(ifm, target, ifm->ifm_mask);
 		if (match == NULL) {
+			mtx_leave(&ifmedia_mtx);
 			ifmedia_add(ifm, target, 0, NULL);
-			match = ifmedia_match(ifm, target, ifm->ifm_mask);
-			if (match == NULL)
+			mtx_enter(&ifmedia_mtx);
+			match = ifmedia_get(ifm, target, ifm->ifm_mask);
+			if (match == NULL) {
+				mtx_leave(&ifmedia_mtx);
 				panic("ifmedia_set failed");
+			}
 		}
 	}
 	ifm->ifm_cur = match;
+	mtx_leave(&ifmedia_mtx);
 
 #ifdef IFMEDIA_DEBUG
 	if (ifmedia_debug) {
@@ -245,8 +253,10 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
 		uint64_t oldmedia;
 		uint64_t newmedia = ifr->ifr_media;
 
-		match = ifmedia_match(ifm, newmedia, ifm->ifm_mask);
+		mtx_enter(&ifmedia_mtx);
+		match = ifmedia_get(ifm, newmedia, ifm->ifm_mask);
 		if (match == NULL) {
+			mtx_leave(&ifmedia_mtx);
 #ifdef IFMEDIA_DEBUG
 			if (ifmedia_debug) {
 				printf("%s: no media found for 0x%llx\n",
@@ -264,8 +274,10 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
 		 */
 		if ((IFM_SUBTYPE(newmedia) != IFM_AUTO) &&
 		    (newmedia == ifm->ifm_media) &&
-		    (match == ifm->ifm_cur))
+		    (match == ifm->ifm_cur)) {
+			mtx_leave(&ifmedia_mtx);
 			return (0);
+		}
 
 		/*
 		 * We found a match, now make the driver switch to it.
@@ -283,10 +295,16 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
 		oldmedia = ifm->ifm_media;
 		ifm->ifm_cur = match;
 		ifm->ifm_media = newmedia;
+		mtx_leave(&ifmedia_mtx);
+
 		error = (*ifm->ifm_change_cb)(ifp);
 		if (error && error != ENETRESET) {
-			ifm->ifm_cur = oldentry;
-			ifm->ifm_media = oldmedia;
+			mtx_enter(&ifmedia_mtx);
+			if (ifm->ifm_cur == match) {
+				ifm->ifm_cur = oldentry;
+				ifm->ifm_media = oldmedia;
+			}
+			mtx_leave(&ifmedia_mtx);
 		}
 		break;
 	}
@@ -302,10 +320,13 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
 		if (ifmr->ifm_count < 0)
 			return (EINVAL);
 
+		mtx_enter(&ifmedia_mtx);
 		ifmr->ifm_active = ifmr->ifm_current = ifm->ifm_cur ?
 		    ifm->ifm_cur->ifm_media : IFM_NONE;
 		ifmr->ifm_mask = ifm->ifm_mask;
 		ifmr->ifm_status = 0;
+		mtx_leave(&ifmedia_mtx);
+
 		(*ifm->ifm_status_cb)(ifp, ifmr);
 
 		mtx_enter(&ifmedia_mtx);
@@ -368,17 +389,30 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
 }
 
 /*
- * Find media entry matching a given ifm word.
+ * Find media entry matching a given ifm word.  Return 1 if found.
  */
-struct ifmedia_entry *
+int
 ifmedia_match(struct ifmedia *ifm, uint64_t target, uint64_t mask)
+{
+	struct ifmedia_entry *match;
+
+	mtx_enter(&ifmedia_mtx);
+	match = ifmedia_get(ifm, target, mask);
+	mtx_leave(&ifmedia_mtx);
+
+	return (match != NULL);
+}
+
+struct ifmedia_entry *
+ifmedia_get(struct ifmedia *ifm, uint64_t target, uint64_t mask)
 {
 	struct ifmedia_entry *match, *next;
 
+	MUTEX_ASSERT_LOCKED(&ifmedia_mtx);
+
 	match = NULL;
 	mask = ~mask;
 
-	mtx_enter(&ifmedia_mtx);
 	TAILQ_FOREACH(next, &ifm->ifm_list, ifm_list) {
 		if ((next->ifm_media & mask) == (target & mask)) {
 			if (match) {
@@ -392,7 +426,6 @@ ifmedia_match(struct ifmedia *ifm, uint64_t target, uint64_t mask)
 			match = next;
 		}
 	}
-	mtx_leave(&ifmedia_mtx);
 
 	return (match);
 }
@@ -417,6 +450,7 @@ ifmedia_delete_instance(struct ifmedia *ifm, uint64_t inst)
 			TAILQ_INSERT_TAIL(&ifmlist, ife, ifm_list);
 		}
 	}
+	ifm->ifm_cur = NULL;
 	mtx_leave(&ifmedia_mtx);
 
 	/* Do not hold mutex longer than necessary, call free() without. */
diff --git a/sys/net/if_media.h b/sys/net/if_media.h
index 6d72f38de60..df5103ccc58 100644
--- a/sys/net/if_media.h
+++ b/sys/net/if_media.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_media.h,v 1.44 2022/07/12 22:08:18 bluhm Exp $	*/
+/*	$OpenBSD: if_media.h,v 1.45 2022/07/14 13:46:25 bluhm Exp $	*/
 /*	$NetBSD: if_media.h,v 1.22 2000/02/17 21:53:16 sommerfeld Exp $	*/
 
 /*-
@@ -103,7 +103,7 @@ TAILQ_HEAD(ifmedia_list, ifmedia_entry);
 struct ifmedia {
 	uint64_t	ifm_mask;	/* mask of changes we don't care about */
 	uint64_t	ifm_media;	/* current user-set media word */
-	struct ifmedia_entry *ifm_cur;	/* currently selected media */
+	struct ifmedia_entry *ifm_cur;	/* [M] currently selected media */
 	struct ifmedia_list ifm_list;	/* [M] list of all supported media */
 	size_t		ifm_nwords;	/* [M] number of ifm_list entries */
 	ifm_change_cb_t	ifm_change_cb;	/* media change driver callback */
@@ -129,7 +129,7 @@ int	ifmedia_ioctl(struct ifnet *, struct ifreq *, struct ifmedia *,
 	    u_long);
 
 /* Locate a media entry */
-struct	ifmedia_entry *ifmedia_match(struct ifmedia *, uint64_t, uint64_t);
+int	ifmedia_match(struct ifmedia *, uint64_t, uint64_t);
 
 /* Delete all media for a given media instance */
 void	ifmedia_delete_instance(struct ifmedia *, uint64_t);