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 2b1547d..dc6fbc3 100644 (file)
--- 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 07cf22c..1566918 100644 (file)
--- 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 6d72f38..df5103c 100644 (file)
--- 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);