Hi,
Protect all writers to ifm_cur with a mutex. ifmedia_match() does
not return any pointers without lock anymore.
ok?
bluhm
Index: dev/ic/if_wi.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/if_wi.c,v
retrieving revision 1.176
diff -u -p -r1.176 if_wi.c
--- dev/ic/if_wi.c 9 Jan 2022 05:42:38 -0000 1.176
+++ dev/ic/if_wi.c 13 Jul 2022 14:15:18 -0000
@@ -2707,7 +2707,7 @@ wi_sync_media(struct wi_softc *sc, int p
}
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;
Index: net/if_media.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_media.c,v
retrieving revision 1.35
diff -u -p -r1.35 if_media.c
--- net/if_media.c 12 Jul 2022 22:27:38 -0000 1.35
+++ net/if_media.c 13 Jul 2022 14:19:29 -0000
@@ -105,6 +105,8 @@ static void ifmedia_printword(uint64_t);
struct mutex ifmedia_mtx = MUTEX_INITIALIZER(IPL_NET);
+struct ifmedia_entry *ifmedia_match_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_
{
struct ifmedia_entry *match;
- match = ifmedia_match(ifm, target, ifm->ifm_mask);
+ mtx_enter(&ifmedia_mtx);
+ match = ifmedia_match_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_
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_match_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_match_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
uint64_t oldmedia;
uint64_t newmedia = ifr->ifr_media;
- match = ifmedia_match(ifm, newmedia, ifm->ifm_mask);
+ mtx_enter(&ifmedia_mtx);
+ match = ifmedia_match_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
*/
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
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
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
}
/*
- * 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_match_get(ifm, target, mask);
+ mtx_leave(&ifmedia_mtx);
+
+ return (match != NULL);
+}
+
+struct ifmedia_entry *
+ifmedia_match_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, uint6
match = next;
}
}
- mtx_leave(&ifmedia_mtx);
return (match);
}
@@ -417,6 +450,7 @@ ifmedia_delete_instance(struct ifmedia *
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. */
Index: net/if_media.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_media.h,v
retrieving revision 1.44
diff -u -p -r1.44 if_media.h
--- net/if_media.h 12 Jul 2022 22:08:18 -0000 1.44
+++ net/if_media.h 13 Jul 2022 14:18:09 -0000
@@ -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
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);
