2013-04-12 17:59:36

by Bing Zhao

[permalink] [raw]
Subject: [PATCH v2 1/2] mwifiex: fix use-after-free in beacon_ie processing

beacon_ie buffer is allocated in mwifiex_fill_new_bss_desc()
and the buffer pointer is saved in bss_desc->beacon_buf.
beacon_ie is freed before the function returns. However,
bss_desc->beacon_buf is still being accessed afterwards.

Fix it by freeing beacon_ie (bss_desc->beacon_buf) in
caller's scope.

Reviewed-by: Doug Anderson <[email protected]>
Reviewed-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
v2: same as v1

drivers/net/wireless/mwifiex/scan.c | 8 ++++++++
drivers/net/wireless/mwifiex/sta_ioctl.c | 14 +++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index e7f6dea..37b24e8 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1533,10 +1533,18 @@ static int mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
/* Make a copy of current BSSID descriptor */
memcpy(&priv->curr_bss_params.bss_descriptor, bss_desc,
sizeof(priv->curr_bss_params.bss_descriptor));
+
+ /* The contents of beacon_ie will be copied to its own buffer
+ * in mwifiex_save_curr_bcn()
+ */
mwifiex_save_curr_bcn(priv);
spin_unlock_irqrestore(&priv->curr_bcn_buf_lock, flags);

done:
+ /* beacon_ie buffer was allocated in function
+ * mwifiex_fill_new_bss_desc(). Free it now.
+ */
+ kfree(bss_desc->beacon_buf);
kfree(bss_desc);
return 0;
}
diff --git a/drivers/net/wireless/mwifiex/sta_ioctl.c b/drivers/net/wireless/mwifiex/sta_ioctl.c
index e6c9b2a..27729cf 100644
--- a/drivers/net/wireless/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/mwifiex/sta_ioctl.c
@@ -140,12 +140,13 @@ int mwifiex_request_set_multicast_list(struct mwifiex_private *priv,
/*
* This function fills bss descriptor structure using provided
* information.
+ * beacon_ie buffer is allocated in this function. It is caller's
+ * responsibility to free the memory.
*/
int mwifiex_fill_new_bss_desc(struct mwifiex_private *priv,
struct cfg80211_bss *bss,
struct mwifiex_bssdescriptor *bss_desc)
{
- int ret;
u8 *beacon_ie;
size_t beacon_ie_len;
struct mwifiex_bss_priv *bss_priv = (void *)bss->priv;
@@ -165,6 +166,7 @@ int mwifiex_fill_new_bss_desc(struct mwifiex_private *priv,

memcpy(bss_desc->mac_address, bss->bssid, ETH_ALEN);
bss_desc->rssi = bss->signal;
+ /* The caller of this function will free beacon_ie */
bss_desc->beacon_buf = beacon_ie;
bss_desc->beacon_buf_size = beacon_ie_len;
bss_desc->beacon_period = bss->beacon_interval;
@@ -182,10 +184,7 @@ int mwifiex_fill_new_bss_desc(struct mwifiex_private *priv,
else
bss_desc->bss_mode = NL80211_IFTYPE_STATION;

- ret = mwifiex_update_bss_desc_with_ie(priv->adapter, bss_desc);
-
- kfree(beacon_ie);
- return ret;
+ return mwifiex_update_bss_desc_with_ie(priv->adapter, bss_desc);
}

static int mwifiex_process_country_ie(struct mwifiex_private *priv,
@@ -349,6 +348,11 @@ int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss,
}

done:
+ /* beacon_ie buffer was allocated in function
+ * mwifiex_fill_new_bss_desc(). Free it now.
+ */
+ if (bss_desc)
+ kfree(bss_desc->beacon_buf);
kfree(bss_desc);
return ret;
}
--
1.7.0.2



2013-04-12 17:39:59

by Bing Zhao

[permalink] [raw]
Subject: [PATCH v2 2/2] mwifiex: remove redundant initialization for bss_descriptor

Initialization of bss_descriptor is unnecessary as the entire
structure will be overwritten by a memcpy followed by.

Initialize disable_11ac flag properly by setting it to true in
mwifiex_fill_new_bss_desc().

Reported-by: Doug Anderson <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
Reviewed-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Yogesh Ashok Powar <[email protected]>
---
v2: Initialize disable_11ac properly (Troy Kisky [email protected])

drivers/net/wireless/mwifiex/scan.c | 29 -----------------------------
drivers/net/wireless/mwifiex/sta_ioctl.c | 5 +++++
2 files changed, 5 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index 37b24e8..9cf5d8f 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1500,36 +1500,7 @@ static int mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
if (ret)
goto done;

- /* Update current bss descriptor parameters */
spin_lock_irqsave(&priv->curr_bcn_buf_lock, flags);
- priv->curr_bss_params.bss_descriptor.bcn_wpa_ie = NULL;
- priv->curr_bss_params.bss_descriptor.wpa_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_rsn_ie = NULL;
- priv->curr_bss_params.bss_descriptor.rsn_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_wapi_ie = NULL;
- priv->curr_bss_params.bss_descriptor.wapi_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_ht_cap = NULL;
- priv->curr_bss_params.bss_descriptor.ht_cap_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_ht_oper = NULL;
- priv->curr_bss_params.bss_descriptor.ht_info_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_bss_co_2040 = NULL;
- priv->curr_bss_params.bss_descriptor.bss_co_2040_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_ext_cap = NULL;
- priv->curr_bss_params.bss_descriptor.ext_cap_offset = 0;
- priv->curr_bss_params.bss_descriptor.beacon_buf = NULL;
- priv->curr_bss_params.bss_descriptor.beacon_buf_size = 0;
- priv->curr_bss_params.bss_descriptor.bcn_vht_cap = NULL;
- priv->curr_bss_params.bss_descriptor.vht_cap_offset = 0;
- priv->curr_bss_params.bss_descriptor.bcn_vht_oper = NULL;
- priv->curr_bss_params.bss_descriptor.vht_info_offset = 0;
- priv->curr_bss_params.bss_descriptor.oper_mode = NULL;
- priv->curr_bss_params.bss_descriptor.oper_mode_offset = 0;
-
- /* Disable 11ac by default. Enable it only where there
- * exist VHT_CAP IE in AP beacon
- */
- priv->curr_bss_params.bss_descriptor.disable_11ac = true;
-
/* Make a copy of current BSSID descriptor */
memcpy(&priv->curr_bss_params.bss_descriptor, bss_desc,
sizeof(priv->curr_bss_params.bss_descriptor));
diff --git a/drivers/net/wireless/mwifiex/sta_ioctl.c b/drivers/net/wireless/mwifiex/sta_ioctl.c
index 27729cf..311d0b2 100644
--- a/drivers/net/wireless/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/mwifiex/sta_ioctl.c
@@ -184,6 +184,11 @@ int mwifiex_fill_new_bss_desc(struct mwifiex_private *priv,
else
bss_desc->bss_mode = NL80211_IFTYPE_STATION;

+ /* Disable 11ac by default. Enable it only where there
+ * exist VHT_CAP IE in AP beacon
+ */
+ bss_desc->disable_11ac = true;
+
return mwifiex_update_bss_desc_with_ie(priv->adapter, bss_desc);
}

--
1.7.0.2