nl80211: Use active_links to notify start/stop state of links

During nl80211_stop_ap(), the link map for all valid links is being
cleared. Following this, in the remove interface sequence, since
valid links are not set, driver_nl80211_link_remove() is called,
which removes the BSS for all links, leading to a dangling pointer
access and causing hostapd to crash.

To address this, introduce a separate active_links link map for
struct i802_bss and update this link map only during nl80211_stop_ap()
and when bringing the link back up. The valid_links link map of
struct i802_bss should be used only during the initialization and
deinitialization of the links.

Fixes: e1bf37022e01 ("nl80211: MLO: Process stop AP event on link basis")
Signed-off-by: Karthikeyan Kathirvel <quic_kathirve@quicinc.com>

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index d54c3e07844d034a5fb5646e0d26903a93ba7c94..326a010e07e8c3a52f4b8774d4a2b7be6bf882e3 100644 (file)
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -2403,6 +2403,7 @@ skip_wifi_status:
         * Use link ID 0 for the single "link" of a non-MLD.
         */
        bss->valid_links = 0;
+       bss->active_links = 0;
        bss->flink = &bss->links[0];
        os_memcpy(bss->flink->addr, bss->addr, ETH_ALEN);
 
@@ -9622,12 +9623,39 @@ fail:
 }
 
 
+void nl80211_update_active_links(struct i802_bss *bss, int link_id)
+{
+       struct i802_link *link = &bss->links[link_id];
+       size_t i;
+
+       wpa_printf(MSG_DEBUG, "nl80211: Update link (ifindex=%d link_id=%u)",
+                  bss->ifindex, link_id);
+
+       if (!(bss->active_links & BIT(link_id))) {
+               wpa_printf(MSG_DEBUG,
+                          "nl80211: MLD: Update link: Link not found");
+               return;
+       }
+
+       wpa_driver_nl80211_del_beacon(bss, link_id);
+
+       bss->active_links &= ~BIT(link_id);
+
+       /* Choose new deflink if we are removing that link */
+       if (bss->flink == link) {
+               for_each_link(bss->active_links, i) {
+                       bss->flink = &bss->links[i];
+                       break;
+               }
+       }
+}
+
+
 int nl80211_remove_link(struct i802_bss *bss, int link_id)
 {
        struct wpa_driver_nl80211_data *drv = bss->drv;
        struct i802_link *link;
        struct nl_msg *msg;
-       size_t i;
        int ret;
        u8 link_addr[ETH_ALEN];
 
@@ -9642,20 +9670,13 @@ int nl80211_remove_link(struct i802_bss *bss, int link_id)
 
        link = &bss->links[link_id];
 
-       wpa_driver_nl80211_del_beacon(bss, link_id);
-
        os_memcpy(link_addr, link->addr, ETH_ALEN);
+
        /* First remove the link locally */
        bss->valid_links &= ~BIT(link_id);
        os_memset(link->addr, 0, ETH_ALEN);
-
-       /* Choose new deflink if we are removing that link */
-       if (bss->flink == link) {
-               for_each_link_default(bss->valid_links, i, 0) {
-                       bss->flink = &bss->links[i];
-                       break;
-               }
-       }
+       /* Clear the active links and set the flink */
+       nl80211_update_active_links(bss, link_id);
 
        /* If this was the last link, reset default link */
        if (!bss->valid_links) {
@@ -14479,7 +14500,7 @@ static int nl80211_link_add(void *priv, u8 link_id, const u8 *addr,
                return -EINVAL;
        }
 
-       if (bss->valid_links & BIT(link_id)) {
+       if (bss->active_links & BIT(link_id)) {
                wpa_printf(MSG_DEBUG,
                           "nl80211: MLD: Link %u already set", link_id);
                return -EINVAL;
@@ -14511,14 +14532,16 @@ static int nl80211_link_add(void *priv, u8 link_id, const u8 *addr,
        os_memcpy(bss->links[link_id].addr, addr, ETH_ALEN);
 
        /* The new link is the first one, make it the default */
-       if (!bss->valid_links)
+       if (!bss->active_links)
                bss->flink = &bss->links[link_id];
 
        bss->valid_links |= BIT(link_id);
+       bss->active_links |= BIT(link_id);
        bss->links[link_id].ctx = bss_ctx;
 
-       wpa_printf(MSG_DEBUG, "nl80211: MLD: valid_links=0x%04x on %s",
-                  bss->valid_links, bss->ifname);
+       wpa_printf(MSG_DEBUG,
+                  "nl80211: MLD: valid_links=0x%04x active_links=0x%04x on %s",
+                  bss->valid_links, bss->active_links, bss->ifname);
 
        if (drv->rtnl_sk)
                rtnl_neigh_add_fdb_entry(bss, addr, true);

diff --git a/src/drivers/driver_nl80211.h b/src/drivers/driver_nl80211.h
index 9c4aedd7acf094667caa65a4b3624a01c0fe80e7..a6f92ff1e5920da9d35f5b9b3df27d22e7ce79c8 100644 (file)
--- a/src/drivers/driver_nl80211.h
+++ b/src/drivers/driver_nl80211.h
@@ -65,7 +65,10 @@ struct i802_bss {
        struct wpa_driver_nl80211_data *drv;
        struct i802_bss *next;
 
+       /* The links which are physically present */
        u16 valid_links;
+       /* The links which are in UP state */
+       u16 active_links;
        struct i802_link links[MAX_NUM_MLD_LINKS];
        struct i802_link *flink, *scan_link;
 
@@ -365,6 +368,7 @@ void nl80211_restore_ap_mode(struct i802_bss *bss);
 struct i802_link * nl80211_get_link(struct i802_bss *bss, s8 link_id);
 u8 nl80211_get_link_id_from_link(struct i802_bss *bss, struct i802_link *link);
 int nl80211_remove_link(struct i802_bss *bss, int link_id);
+void nl80211_update_active_links(struct i802_bss *bss, int link_id);
 
 static inline bool nl80211_link_valid(u16 links, s8 link_id)
 {

diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c
index cb37a500449f2292c172bb5693e85ab2292deea0..40555c9577e4d0643a113ef0210e8c8cff60a38c 100644 (file)
--- a/src/drivers/driver_nl80211_event.c
+++ b/src/drivers/driver_nl80211_event.c
@@ -2467,10 +2467,8 @@ static void nl80211_stop_ap(struct i802_bss *bss, struct nlattr **tb)
                           "nl80211: STOP_AP event on link %d", link_id);
                ctx = mld_link->ctx;
 
-               /* The driver would have already deleted the link and this call
-                * will return an error. Ignore that since nl80211_remove_link()
-                * is called here only to update the bss->links[] state. */
-               nl80211_remove_link(bss, link_id);
+               /* Bring down the active link */
+               nl80211_update_active_links(bss, link_id);
        }
 
        wpa_supplicant_event(ctx, EVENT_INTERFACE_UNAVAILABLE, NULL);