From c8361ee8a7a8a57af5593f6dda50515e6b7f7998 Mon Sep 17 00:00:00 2001 From: Bing Zhao Date: Thu, 9 May 2013 11:55:28 -0700 Subject: [PATCH] mwifiex: remove global user_scan_cfg variable As the variable is used only for preparation of internal scan commands, we don't need to keep it allocated until the entire scan completes. We will define it as a local variable and free immediately after it's use. New flag 'scan_aborting' is added to handle race between mwifiex_close() and scan handler. Previously user_scan_cfg pointer used to take care of this. This patch fixes a memory leak in mwifiex_cfg80211_scan after running "iwlist mlan0 scan & sleep 1; rmmod mwifiex_sdio". BUG=None TEST="iwlist mlan0 scan & sleep 1; rmmod mwifiex_sdio"; "echo scan > /sys/kernel/debug/kmemleak; cat /sys/kernel/debug/kmemleak" Change-Id: I3ab5aacf1bec957c6279a784d9ed557bf0691f2c Reported-by: Daniel Drake [OLPC] Tested-by: Daniel Drake [OLPC] Signed-off-by: Amitkumar Karwar Signed-off-by: Bing Zhao Reviewed-on: https://gerrit.chromium.org/gerrit/50686 Reviewed-by: Paul Stewart --- drivers/net/wireless/mwifiex/cfg80211.c | 31 ++++++++++++++----------- drivers/net/wireless/mwifiex/init.c | 20 ++++++---------- drivers/net/wireless/mwifiex/main.c | 1 + drivers/net/wireless/mwifiex/main.h | 2 +- drivers/net/wireless/mwifiex/scan.c | 22 +++++++----------- 5 files changed, 34 insertions(+), 42 deletions(-) diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c index 54cb8c6da63b..4dcb8fbac133 100644 --- a/drivers/net/wireless/mwifiex/cfg80211.c +++ b/drivers/net/wireless/mwifiex/cfg80211.c @@ -1352,6 +1352,7 @@ mwifiex_cfg80211_scan(struct wiphy *wiphy, struct net_device *dev, struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); int i, ret; struct ieee80211_channel *chan; + struct mwifiex_user_scan_cfg *user_scan_cfg; wiphy_dbg(wiphy, "info: received scan request on %s\n", dev->name); @@ -1362,22 +1363,24 @@ mwifiex_cfg80211_scan(struct wiphy *wiphy, struct net_device *dev, return -EBUSY; } - if (priv->user_scan_cfg) { + /* Block scan request if scan operation or scan cleanup when interface + * is disabled is in process + */ + if (priv->scan_request || priv->scan_aborting) { dev_err(priv->adapter->dev, "cmd: Scan already in process..\n"); return -EBUSY; } - priv->user_scan_cfg = kzalloc(sizeof(struct mwifiex_user_scan_cfg), - GFP_KERNEL); - if (!priv->user_scan_cfg) { + user_scan_cfg = kzalloc(sizeof(*user_scan_cfg), GFP_KERNEL); + if (!user_scan_cfg) { dev_err(priv->adapter->dev, "failed to alloc scan_req\n"); return -ENOMEM; } priv->scan_request = request; - priv->user_scan_cfg->num_ssids = request->n_ssids; - priv->user_scan_cfg->ssid_list = request->ssids; + user_scan_cfg->num_ssids = request->n_ssids; + user_scan_cfg->ssid_list = request->ssids; if (request->ie && request->ie_len) { for (i = 0; i < MWIFIEX_MAX_VSIE_NUM; i++) { @@ -1392,25 +1395,25 @@ mwifiex_cfg80211_scan(struct wiphy *wiphy, struct net_device *dev, for (i = 0; i < request->n_channels; i++) { chan = request->channels[i]; - priv->user_scan_cfg->chan_list[i].chan_number = chan->hw_value; - priv->user_scan_cfg->chan_list[i].radio_type = chan->band; + user_scan_cfg->chan_list[i].chan_number = chan->hw_value; + user_scan_cfg->chan_list[i].radio_type = chan->band; if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN) - priv->user_scan_cfg->chan_list[i].scan_type = + user_scan_cfg->chan_list[i].scan_type = MWIFIEX_SCAN_TYPE_PASSIVE; else - priv->user_scan_cfg->chan_list[i].scan_type = + user_scan_cfg->chan_list[i].scan_type = MWIFIEX_SCAN_TYPE_ACTIVE; - priv->user_scan_cfg->chan_list[i].scan_time = 0; + user_scan_cfg->chan_list[i].scan_time = 0; } - ret = mwifiex_scan_networks(priv, priv->user_scan_cfg); + ret = mwifiex_scan_networks(priv, user_scan_cfg); + kfree(user_scan_cfg); if (ret) { dev_err(priv->adapter->dev, "scan failed: %d\n", ret); + priv->scan_aborting = false; priv->scan_request = NULL; - kfree(priv->user_scan_cfg); - priv->user_scan_cfg = NULL; return ret; } diff --git a/drivers/net/wireless/mwifiex/init.c b/drivers/net/wireless/mwifiex/init.c index 0ca260b8408c..ea90e9d5dc46 100644 --- a/drivers/net/wireless/mwifiex/init.c +++ b/drivers/net/wireless/mwifiex/init.c @@ -87,19 +87,13 @@ static void scan_delay_timer_fn(unsigned long data) adapter->empty_tx_q_cnt = 0; spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags); - if (priv->user_scan_cfg) { - if (priv->scan_request) { - dev_dbg(priv->adapter->dev, - "info: aborting scan\n"); - cfg80211_scan_done(priv->scan_request, 1); - priv->scan_request = NULL; - } else { - dev_dbg(priv->adapter->dev, - "info: scan already aborted\n"); - } - - kfree(priv->user_scan_cfg); - priv->user_scan_cfg = NULL; + if (priv->scan_request) { + dev_dbg(adapter->dev, "info: aborting scan\n"); + cfg80211_scan_done(priv->scan_request, 1); + priv->scan_request = NULL; + } else { + priv->scan_aborting = false; + dev_dbg(adapter->dev, "info: scan already aborted\n"); } goto done; } diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c index 0a95b462eb01..cf0c5fc7c2d2 100644 --- a/drivers/net/wireless/mwifiex/main.c +++ b/drivers/net/wireless/mwifiex/main.c @@ -411,6 +411,7 @@ mwifiex_close(struct net_device *dev) dev_dbg(priv->adapter->dev, "aborting scan on ndo_stop\n"); cfg80211_scan_done(priv->scan_request, 1); priv->scan_request = NULL; + priv->scan_aborting = true; } return 0; diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h index 1e3f5aa27d54..315f07e02381 100644 --- a/drivers/net/wireless/mwifiex/main.h +++ b/drivers/net/wireless/mwifiex/main.h @@ -467,7 +467,6 @@ struct mwifiex_private { struct semaphore async_sem; u8 report_scan_result; struct cfg80211_scan_request *scan_request; - struct mwifiex_user_scan_cfg *user_scan_cfg; u8 cfg_bssid[6]; u8 country_code[IEEE80211_COUNTRY_STRING_LEN]; struct wps wps; @@ -479,6 +478,7 @@ struct mwifiex_private { struct mwifiex_ds_misc_subsc_evt async_subsc_evt_storage; u32 mgmt_frame_mask; u32 mgmt_rx_freq; + bool scan_aborting; }; enum mwifiex_ba_status { diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c index dbd08271ba2d..5ff33b4ed0c7 100644 --- a/drivers/net/wireless/mwifiex/scan.c +++ b/drivers/net/wireless/mwifiex/scan.c @@ -1743,22 +1743,16 @@ check_next_scan: if (priv->report_scan_result) priv->report_scan_result = false; - if (priv->user_scan_cfg) { - if (priv->scan_request) { - dev_dbg(priv->adapter->dev, - "info: notifying scan done\n"); - cfg80211_scan_done(priv->scan_request, 0); - priv->scan_request = NULL; - } else { - dev_dbg(priv->adapter->dev, - "info: scan already aborted\n"); - } - - kfree(priv->user_scan_cfg); - priv->user_scan_cfg = NULL; + if (priv->scan_request) { + dev_dbg(adapter->dev, "info: notifying scan done\n"); + cfg80211_scan_done(priv->scan_request, 0); + priv->scan_request = NULL; + } else { + priv->scan_aborting = false; + dev_dbg(adapter->dev, "info: scan already aborted\n"); } } else { - if (priv->user_scan_cfg && !priv->scan_request) { + if (priv->scan_aborting && !priv->scan_request) { spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags); adapter->scan_delay_cnt = MWIFIEX_MAX_SCAN_DELAY_CNT; -- 2.20.1