From 1cc9250c5648d951ba5a9a5aa95f02018b9ce1f6 Mon Sep 17 00:00:00 2001 From: bluhm Date: Tue, 12 Jul 2022 22:08:17 +0000 Subject: [PATCH] Protect interface media list with a mutex. This is just a start to make make media structures MP safe. OK mvs@ --- sys/dev/ic/gem.c | 4 ++- sys/net/if_media.c | 85 +++++++++++++++++++++++++++++++++------------- sys/net/if_media.h | 12 +++++-- 3 files changed, 74 insertions(+), 27 deletions(-) diff --git a/sys/dev/ic/gem.c b/sys/dev/ic/gem.c index cce22d35ff8..42bdeb9cb68 100644 --- a/sys/dev/ic/gem.c +++ b/sys/dev/ic/gem.c @@ -1,4 +1,4 @@ -/* $OpenBSD: gem.c,v 1.126 2020/12/12 11:48:52 jan Exp $ */ +/* $OpenBSD: gem.c,v 1.127 2022/07/12 22:08:17 bluhm Exp $ */ /* $NetBSD: gem.c,v 1.1 2001/09/16 00:11:43 eeh Exp $ */ /* @@ -332,6 +332,7 @@ gem_config(struct gem_softc *sc) } /* Check if we support GigE media. */ + mtx_enter(&ifmedia_mtx); TAILQ_FOREACH(ifm, &sc->sc_media.ifm_list, ifm_list) { if (IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_T || IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_SX || @@ -341,6 +342,7 @@ gem_config(struct gem_softc *sc) break; } } + mtx_leave(&ifmedia_mtx); /* Attach the interface. */ if_attach(ifp); diff --git a/sys/net/if_media.c b/sys/net/if_media.c index 3173bda4696..05e0523b331 100644 --- a/sys/net/if_media.c +++ b/sys/net/if_media.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_media.c,v 1.33 2022/07/10 21:13:41 bluhm Exp $ */ +/* $OpenBSD: if_media.c,v 1.34 2022/07/12 22:08:18 bluhm Exp $ */ /* $NetBSD: if_media.c,v 1.10 2000/03/13 23:52:39 soren Exp $ */ /*- @@ -82,6 +82,7 @@ #include #include #include +#include #include #ifdef IFMEDIA_DEBUG @@ -102,6 +103,8 @@ int ifmedia_debug = 0; static void ifmedia_printword(uint64_t); #endif +struct mutex ifmedia_mtx = MUTEX_INITIALIZER(IPL_NET); + /* * Initialize if_media struct for a specific interface instance. */ @@ -110,6 +113,7 @@ ifmedia_init(struct ifmedia *ifm, uint64_t dontcare_mask, ifm_change_cb_t change_callback, ifm_stat_cb_t status_callback) { TAILQ_INIT(&ifm->ifm_list); + ifm->ifm_nwords = 0; ifm->ifm_cur = NULL; ifm->ifm_media = 0; ifm->ifm_mask = dontcare_mask; /* IF don't-care bits */ @@ -145,7 +149,10 @@ ifmedia_add(struct ifmedia *ifm, uint64_t mword, int data, void *aux) entry->ifm_data = data; entry->ifm_aux = aux; + mtx_enter(&ifmedia_mtx); TAILQ_INSERT_TAIL(&ifm->ifm_list, entry, ifm_list); + ifm->ifm_nwords++; + mtx_leave(&ifmedia_mtx); } /* @@ -290,7 +297,6 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, case SIOCGIFMEDIA: { struct ifmediareq *ifmr = (struct ifmediareq *) ifr; - struct ifmedia_entry *ep; size_t nwords; if (ifmr->ifm_count < 0) @@ -302,37 +308,54 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, ifmr->ifm_status = 0; (*ifm->ifm_status_cb)(ifp, ifmr); - /* - * Count them so we know a-priori how much is the max we'll - * need. - */ - ep = TAILQ_FIRST(&ifm->ifm_list); - for (nwords = 0; ep != NULL; ep = TAILQ_NEXT(ep, ifm_list)) - nwords++; + mtx_enter(&ifmedia_mtx); + nwords = ifm->ifm_nwords; + mtx_leave(&ifmedia_mtx); - if (ifmr->ifm_count != 0) { - size_t minwords, ksiz; + if (ifmr->ifm_count == 0) { + ifmr->ifm_count = nwords; + return (0); + } + + do { + struct ifmedia_entry *ife; uint64_t *kptr; + size_t ksiz; - minwords = nwords > (size_t)ifmr->ifm_count ? - (size_t)ifmr->ifm_count : nwords; kptr = mallocarray(nwords, sizeof(*kptr), M_TEMP, M_WAITOK | M_ZERO); ksiz = nwords * sizeof(*kptr); + + mtx_enter(&ifmedia_mtx); + /* Media list might grow during malloc(). */ + if (nwords < ifm->ifm_nwords) { + nwords = ifm->ifm_nwords; + mtx_leave(&ifmedia_mtx); + free(kptr, M_TEMP, ksiz); + continue; + } + /* Request memory too small, set error and ifm_count. */ + if (ifmr->ifm_count < ifm->ifm_nwords) { + nwords = ifm->ifm_nwords; + mtx_leave(&ifmedia_mtx); + free(kptr, M_TEMP, ksiz); + error = E2BIG; + break; + } /* * Get the media words from the interface's list. */ - ep = TAILQ_FIRST(&ifm->ifm_list); - for (nwords = 0; ep != NULL && nwords < minwords; - ep = TAILQ_NEXT(ep, ifm_list)) - kptr[nwords++] = ep->ifm_media; - if (ep == NULL) - error = copyout(kptr, ifmr->ifm_ulist, - nwords * sizeof(*kptr)); - else - error = E2BIG; + nwords = 0; + TAILQ_FOREACH(ife, &ifm->ifm_list, ifm_list) { + kptr[nwords++] = ife->ifm_media; + } + KASSERT(nwords == ifm->ifm_nwords); + mtx_leave(&ifmedia_mtx); + + error = copyout(kptr, ifmr->ifm_ulist, + nwords * sizeof(*kptr)); free(kptr, M_TEMP, ksiz); - } + } while (0); ifmr->ifm_count = nwords; break; } @@ -355,6 +378,7 @@ ifmedia_match(struct ifmedia *ifm, uint64_t target, uint64_t mask) 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) { @@ -368,6 +392,7 @@ ifmedia_match(struct ifmedia *ifm, uint64_t target, uint64_t mask) match = next; } } + mtx_leave(&ifmedia_mtx); return (match); } @@ -379,14 +404,26 @@ void ifmedia_delete_instance(struct ifmedia *ifm, uint64_t inst) { struct ifmedia_entry *ife, *nife; + struct ifmedia_list ifmlist; + + TAILQ_INIT(&ifmlist); + mtx_enter(&ifmedia_mtx); TAILQ_FOREACH_SAFE(ife, &ifm->ifm_list, ifm_list, nife) { if (inst == IFM_INST_ANY || inst == IFM_INST(ife->ifm_media)) { TAILQ_REMOVE(&ifm->ifm_list, ife, ifm_list); - free(ife, M_IFADDR, sizeof *ife); + ifm->ifm_nwords--; + TAILQ_INSERT_TAIL(&ifmlist, ife, ifm_list); } } + mtx_leave(&ifmedia_mtx); + + /* Do not hold mutex longer than necessary, call free() without. */ + while((ife = TAILQ_FIRST(&ifmlist)) != NULL) { + TAILQ_REMOVE(&ifmlist, ife, ifm_list); + free(ife, M_IFADDR, sizeof *ife); + } } /* diff --git a/sys/net/if_media.h b/sys/net/if_media.h index 74278b0e1cf..6d72f38de60 100644 --- a/sys/net/if_media.h +++ b/sys/net/if_media.h @@ -1,4 +1,4 @@ -/* $OpenBSD: if_media.h,v 1.43 2022/07/10 21:13:41 bluhm Exp $ */ +/* $OpenBSD: if_media.h,v 1.44 2022/07/12 22:08:18 bluhm Exp $ */ /* $NetBSD: if_media.h,v 1.22 2000/02/17 21:53:16 sommerfeld Exp $ */ /*- @@ -71,6 +71,7 @@ #ifdef _KERNEL struct ifnet; +extern struct mutex ifmedia_mtx; #include /* @@ -79,6 +80,11 @@ struct ifnet; typedef int (*ifm_change_cb_t)(struct ifnet *); typedef void (*ifm_stat_cb_t)(struct ifnet *, struct ifmediareq *); +/* + * Locks used to protect struct members in this file: + * M ifmedia_mtx interface media mutex + */ + /* * In-kernel representation of a single supported media type. */ @@ -89,6 +95,7 @@ struct ifmedia_entry { void *ifm_aux; /* for driver-specific use */ }; +TAILQ_HEAD(ifmedia_list, ifmedia_entry); /* * One of these goes into a network interface's softc structure. * It is used to keep general media state. @@ -97,7 +104,8 @@ 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 */ - TAILQ_HEAD(, ifmedia_entry) ifm_list; /* list of all supported 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 */ ifm_stat_cb_t ifm_status_cb; /* media status driver callback */ }; -- 2.20.1