]> git.ipfire.org Git - thirdparty/hostap.git/commitdiff
nl80211: MLD: Fix is_shared_drv ops logic when num links is one
authorGanesh Kariganuru Mahabalesh <quic_gkarigan@quicinc.com>
Thu, 25 Apr 2024 10:15:25 +0000 (15:45 +0530)
committerJouni Malinen <j@w1.fi>
Fri, 9 Aug 2024 07:05:39 +0000 (10:05 +0300)
Whenever there is only one BSS left and if the number of links is one,
is_shared_drv() returns false assuming no one else is sharing the driver
interface. However, when the number of links is one, this does not
guarantee that the caller's link ID is the only active link ID. If this
is not the case and false is returned, the caller calls hapd_deinit()
which will free the driver interface. However, when the actual active
link_id reaches deinit path, this leads to dereferencing a NULL pointer
ultimately leading to segmentation fault.

To prevent this, pass the link ID into the is_drv_shared() ops and match
it with only with active link IDs. Only return false if they are same.

Signed-off-by: Ganesh Kariganuru Mahabalesh <quic_gkarigan@quicinc.com>
Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
src/ap/hostapd.c
src/drivers/driver.h
src/drivers/driver_nl80211.c

index d46358bba46d6ca145fc0ba4357c895f2a2ce265..57ae12d4a1e929b2b081f7fca60527acf9a56aae 100644 (file)
@@ -3400,7 +3400,8 @@ static void hostapd_cleanup_driver(const struct wpa_driver_ops *driver,
                driver->hapd_deinit(drv_priv);
        } else if (hostapd_mld_is_first_bss(iface->bss[0]) &&
                   driver->is_drv_shared &&
-                  !driver->is_drv_shared(drv_priv)) {
+                  !driver->is_drv_shared(drv_priv,
+                                         iface->bss[0]->mld_link_id)) {
                driver->hapd_deinit(drv_priv);
                hostapd_mld_interface_freed(iface->bss[0]);
        } else if (hostapd_if_link_remove(iface->bss[0],
index f7beed8c0a8205a5302b53849d5fe5bc84ac7bec..c4cf46c30508178e0162163eeb723f39650195f1 100644 (file)
@@ -5196,14 +5196,19 @@ struct wpa_driver_ops {
        /**
         * is_drv_shared - Check whether the driver interface is shared
         * @priv: Private driver interface data from init()
+        * @link_id: Link ID to match
+        * Returns: true if it is being used or else false.
         *
         * Checks whether the driver interface is being used by other partner
         * BSS(s) or not. This is used to decide whether the driver interface
         * needs to be deinitilized when one interface is getting deinitialized.
         *
-        * Returns: true if it is being used or else false.
+        * NOTE: @link_id will be used only when there is only one BSS
+        * present and if that single link is active. In that case, the
+        * link ID is matched with the active link_id to decide whether the
+        * driver interface is being used by other partner BSS(s).
         */
-       bool (*is_drv_shared)(void *priv);
+       bool (*is_drv_shared)(void *priv, int link_id);
 
        /**
         * link_sta_remove - Remove a link STA from an MLD STA
index 592e66fd68569533db2e416802e718da7b341a6b..b729b3e5b0054c58a7088518f4a3c9675aa2bc8b 100644 (file)
@@ -10797,11 +10797,13 @@ static int driver_nl80211_link_remove(void *priv, enum wpa_driver_if_type type,
 }
 
 
-static bool nl80211_is_drv_shared(void *priv)
+static bool nl80211_is_drv_shared(void *priv, int link_id)
 {
        struct i802_bss *bss = priv;
        struct wpa_driver_nl80211_data *drv = bss->drv;
-       unsigned int num_bss = 0;
+       unsigned int num_bss = 0, num_links = 0;
+       bool self = false;
+       u8 i;
 
        /* If any other BSS exist, someone else is using this since at this
         * time, we would have removed all BSSs created by this driver and only
@@ -10816,13 +10818,23 @@ static bool nl80211_is_drv_shared(void *priv)
        /* This is the only BSS present */
        bss = priv;
 
-       /* If only one/no link is there no one is sharing */
-       if (bss->valid_links <= 1)
+       for_each_link(bss->valid_links, i) {
+               num_links++;
+               if (i == link_id)
+                       self = true;
+       }
+
+       /* More than one links means some one is still sharing */
+       if (num_links > 1)
+               return true;
+
+       /* Even if only one link is there, it should match the given
+        * link ID to assert that no one else is sharing. */
+       if (num_links == 1 && self)
                return false;
 
-       /* More than one link means someone is still using. To check if
-        * only 1 bit is set, power of 2 condition can be checked. */
-       if (!(bss->valid_links & (bss->valid_links - 1)))
+       /* No links are active means no one is sharing */
+       if (num_links == 0)
                return false;
 
        return true;