[src-draft/trunk]: src/sys/net80211 As VAPs are MPSAFE, we can not touch if_f...

[Martin Husemann <martin%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src-all/rev/cfa9db706e95
branches:  trunk
changeset: 377136:cfa9db706e95
user:      Martin Husemann <martin%NetBSD.org@localhost>
date:      Tue Sep 20 20:53:48 2022 +0200

description:
As VAPs are MPSAFE, we can not touch if_flags w/o holding IFNET_LOCK

diffstat:

 sys/net80211/ieee80211_ioctl.c |  27 +++++++++++++++++++++++++++
 sys/net80211/ieee80211_proto.c |  28 +++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 3 deletions(-)

diffs (120 lines):

diff -r 1f7048451ca1 -r cfa9db706e95 sys/net80211/ieee80211_ioctl.c
--- a/sys/net80211/ieee80211_ioctl.c    Tue Sep 20 20:52:26 2022 +0200
+++ b/sys/net80211/ieee80211_ioctl.c    Tue Sep 20 20:53:48 2022 +0200
@@ -3768,7 +3768,30 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
                        if (vap->iv_state == IEEE80211_S_INIT) {
                                if (ic->ic_nrunning == 0)
                                        wait = 1;
+
+#if __NetBSD__
+                               /*
+                                * The first activation will be defered to
+                                * ic_parent_task,
+                                * do not mark RUNNING prematurely.
+                                * For later VAPs we can do it before calling
+                                * ieee80211_start_locked
+                                * and avoid some tricky races.
+                                */
+                               if (!wait) {
+                                       KASSERTMSG(IFNET_LOCKED(ifp), "%s",
+                                           ifp->if_xname);
+                                       ifp->if_flags |= IFF_RUNNING;
+                               }
+#endif
                                ieee80211_start_locked(vap);
+#if __NetBSD__
+                               if (wait) {
+                                       KASSERTMSG(IFNET_LOCKED(ifp), "%s",
+                                           ifp->if_xname);
+                                       ifp->if_flags |= IFF_RUNNING;
+                               }
+#endif
                        }
 #if __FreeBSD__
                } else if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
@@ -3782,6 +3805,10 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
                        if (ic->ic_nrunning == 1)
                                wait = 1;
                        ieee80211_stop_locked(vap);
+#if __NetBSD__
+                       KASSERTMSG(IFNET_LOCKED(ifp), "%s", ifp->if_xname);
+                       ifp->if_flags &= ~IFF_RUNNING;
+#endif
                }
                IEEE80211_UNLOCK(ic);
                /* Wait for parent ioctl handler if it was queued */
diff -r 1f7048451ca1 -r cfa9db706e95 sys/net80211/ieee80211_proto.c
--- a/sys/net80211/ieee80211_proto.c    Tue Sep 20 20:52:26 2022 +0200
+++ b/sys/net80211/ieee80211_proto.c    Tue Sep 20 20:53:48 2022 +0200
@@ -1690,7 +1690,12 @@ ieee80211_start_locked(struct ieee80211v
                ieee80211_notify_ifnet_change(vap);
 #elif __NetBSD__
        if ((ifp->if_flags & IFF_RUNNING) == 0) {
-               ifp->if_flags |= IFF_RUNNING;
+               /*
+                * Caller will mark us running if this is during if_init -
+                * otherwise we can not get IFNET_LOCK(ifp) here, so
+                * are not allowed to touch if_flags.
+                * We assert this never happens below.
+                */
 #endif
 
                /*
@@ -1708,6 +1713,9 @@ ieee80211_start_locked(struct ieee80211v
                        return;
                }
        }
+#if __NetBSD__
+       KASSERT(ifp->if_flags & IFF_RUNNING);
+#endif
        /*
         * If the parent is up and running, then kick the
         * 802.11 state machine as appropriate.
@@ -1772,6 +1780,7 @@ ieee80211_init(struct ifnet *ifp)
 {
        struct ieee80211vap *vap = ifp->if_softc;
        static ONCE_DECL(ieee80211_init_once);
+       bool first_one;
 
        IEEE80211_DPRINTF(vap, IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG,
            "%s\n", __func__);
@@ -1779,7 +1788,19 @@ ieee80211_init(struct ifnet *ifp)
        RUN_ONCE(&ieee80211_init_once, ieee80211_init0);
 
        IEEE80211_LOCK(vap->iv_ic);
+       KASSERT(IFNET_LOCKED(ifp));
+       first_one = vap->iv_ic->ic_nrunning <= 0;
+       /*
+        * The first activation will be defered to ic_parent_task,
+        * do not mark RUNNING prematurely.
+        * For later VAPs we can do it before calling ieee80211_start_locked
+        * and avoid some tricky races.
+        */
+       if (!first_one)
+               ifp->if_flags |= IFF_RUNNING;
        ieee80211_start_locked(vap);
+       if (first_one)
+               ifp->if_flags |= IFF_RUNNING;
        IEEE80211_UNLOCK(vap->iv_ic);
        return 0;
 }
@@ -1796,8 +1817,10 @@ ieee80211_start_all(struct ieee80211com 
        IEEE80211_LOCK(ic);
        TAILQ_FOREACH(vap, &ic->ic_vaps, iv_next) {
                struct ifnet *ifp = vap->iv_ifp;
-               if (IFNET_IS_UP_RUNNING(ifp))   /* NB: avoid recursion */
+
+               if (IFNET_IS_UP_RUNNING(ifp))   /* NB: avoid recursion */ {
                        ieee80211_start_locked(vap);
+               }
        }
        IEEE80211_UNLOCK(ic);
 }
@@ -1827,7 +1850,6 @@ ieee80211_stop_locked(struct ieee80211va
                ieee80211_notify_ifnet_change(vap);
 #elif __NetBSD__
        if (ifp->if_flags & IFF_RUNNING) {
-               ifp->if_flags &= ~IFF_RUNNING;  /* mark us stopped */
 #endif
                if (--ic->ic_nrunning == 0) {
                        IEEE80211_DPRINTF(vap,