From bfdf1fd98d7d2bc1eec347ba2bc287dadedf504c Mon Sep 17 00:00:00 2001 From: Alexander Bluhm Date: Thu, 14 Jul 2022 13:46:26 +0000 Subject: Protect all writers to ifm_cur with a mutex. ifmedia_match() does not return any pointers without lock anymore. OK mvs@ mbuhl@ --- sys/dev/ic/if_wi.c | 4 ++-- sys/net/if_media.c | 60 ++++++++++++++++++++++++++++++++++++++++++------------ sys/net/if_media.h | 6 +++--- 3 files changed, 52 insertions(+), 18 deletions(-) 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); -- cgit v1.2.3