* [PATCH 3.13 2/6] mwifiex: failure path handling in mwifiex_add_virtual_intf()
2013-11-15 3:10 [PATCH 3.13 1/6] mwifiex: use return value of mwifiex_add_virtual_intf() correctly Bing Zhao
@ 2013-11-15 3:10 ` Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 3/6] mwifiex: fix NULL pointer dereference in mwifiex_fw_dpc Bing Zhao
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Bing Zhao @ 2013-11-15 3:10 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, Amitkumar Karwar, Ujjal Roy, Avinash Patil,
Nishant Sarmukadam, Frank Huang, Bing Zhao
From: Amitkumar Karwar <akarwar@marvell.com>
1) If register_netdevice() is failed, we are freeing netdev
pointer, but priv->netdev is not cleared. This gives kernel
paging request error when driver is unloaded or interface is
deleted. Fix the problem by clearing the pointer.
2) Fix memory leak issue by freeing 'wdev' in failure paths.
Also, clear priv->wdev pointer.
As mwifiex_add_virtual_intf() successfully handles the
failure conditions, redundant code under err_add_intf label
is removed in this patch.
Reported-by: Ujjal Roy <royujjal@gmail.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/mwifiex/cfg80211.c | 20 ++++++++++++++++----
drivers/net/wireless/mwifiex/main.c | 13 ++-----------
2 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index fbad00a..ccc9c08 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -2210,8 +2210,10 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
priv->bss_started = 0;
priv->bss_num = 0;
- if (mwifiex_cfg80211_init_p2p_client(priv))
- return ERR_PTR(-EFAULT);
+ if (mwifiex_cfg80211_init_p2p_client(priv)) {
+ wdev = ERR_PTR(-EFAULT);
+ goto done;
+ }
break;
default:
@@ -2224,7 +2226,8 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
if (!dev) {
wiphy_err(wiphy, "no memory available for netdevice\n");
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
- return ERR_PTR(-ENOMEM);
+ wdev = ERR_PTR(-ENOMEM);
+ goto done;
}
mwifiex_init_priv_params(priv, dev);
@@ -2264,7 +2267,9 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
wiphy_err(wiphy, "cannot register virtual network device\n");
free_netdev(dev);
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
- return ERR_PTR(-EFAULT);
+ priv->netdev = NULL;
+ wdev = ERR_PTR(-EFAULT);
+ goto done;
}
sema_init(&priv->async_sem, 1);
@@ -2274,6 +2279,13 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
#ifdef CONFIG_DEBUG_FS
mwifiex_dev_debugfs_init(priv);
#endif
+
+done:
+ if (IS_ERR(wdev)) {
+ kfree(priv->wdev);
+ priv->wdev = NULL;
+ }
+
return wdev;
}
EXPORT_SYMBOL_GPL(mwifiex_add_virtual_intf);
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 7c7da3e..9236b42 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -411,7 +411,7 @@ static void mwifiex_terminate_workqueue(struct mwifiex_adapter *adapter)
*/
static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
{
- int ret, i;
+ int ret;
char fmt[64];
struct mwifiex_private *priv;
struct mwifiex_adapter *adapter = context;
@@ -479,6 +479,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
NL80211_IFTYPE_STATION, NULL, NULL);
if (IS_ERR(wdev)) {
dev_err(adapter->dev, "cannot create default STA interface\n");
+ rtnl_unlock();
goto err_add_intf;
}
rtnl_unlock();
@@ -488,16 +489,6 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
goto done;
err_add_intf:
- for (i = 0; i < adapter->priv_num; i++) {
- priv = adapter->priv[i];
-
- if (!priv)
- continue;
-
- if (priv->wdev && priv->netdev)
- mwifiex_del_virtual_intf(adapter->wiphy, priv->wdev);
- }
- rtnl_unlock();
err_register_cfg80211:
wiphy_unregister(adapter->wiphy);
wiphy_free(adapter->wiphy);
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3.13 3/6] mwifiex: fix NULL pointer dereference in mwifiex_fw_dpc
2013-11-15 3:10 [PATCH 3.13 1/6] mwifiex: use return value of mwifiex_add_virtual_intf() correctly Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 2/6] mwifiex: failure path handling in mwifiex_add_virtual_intf() Bing Zhao
@ 2013-11-15 3:10 ` Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 4/6] mwifiex: fix potential mem leak in .del_virtual_intf Bing Zhao
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Bing Zhao @ 2013-11-15 3:10 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, Amitkumar Karwar, Ujjal Roy, Avinash Patil,
Nishant Sarmukadam, Frank Huang, Bing Zhao
From: Amitkumar Karwar <akarwar@marvell.com>
We don't need to free/unregister wiphy when
mwifiex_register_cfg80211() fails. The routine internally takes
care of it. This redundant code can cause NULL pointer dereference,
for adapter->wiphy.
Reported-by: Ujjal Roy <royujjal@gmail.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/mwifiex/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 9236b42..42d9a68 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -470,7 +470,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
priv = adapter->priv[MWIFIEX_BSS_ROLE_STA];
if (mwifiex_register_cfg80211(adapter)) {
dev_err(adapter->dev, "cannot register with cfg80211\n");
- goto err_register_cfg80211;
+ goto err_init_fw;
}
rtnl_lock();
@@ -489,7 +489,6 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
goto done;
err_add_intf:
-err_register_cfg80211:
wiphy_unregister(adapter->wiphy);
wiphy_free(adapter->wiphy);
err_init_fw:
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3.13 4/6] mwifiex: fix potential mem leak in .del_virtual_intf
2013-11-15 3:10 [PATCH 3.13 1/6] mwifiex: use return value of mwifiex_add_virtual_intf() correctly Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 2/6] mwifiex: failure path handling in mwifiex_add_virtual_intf() Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 3/6] mwifiex: fix NULL pointer dereference in mwifiex_fw_dpc Bing Zhao
@ 2013-11-15 3:10 ` Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 5/6] mwifiex: fix memory leak issue for sdio and pcie cards Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 6/6] mwifiex: fix issues in driver unload path for USB chipsets Bing Zhao
4 siblings, 0 replies; 6+ messages in thread
From: Bing Zhao @ 2013-11-15 3:10 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, Amitkumar Karwar, Ujjal Roy, Avinash Patil,
Nishant Sarmukadam, Frank Huang, Bing Zhao
From: Amitkumar Karwar <akarwar@marvell.com>
1) Currently we freeing wdev for each interface in driver unload
path. We may leak memory if user have already deleted an interface.
mwifiex_add_virtual_intf() allocates wdev structure. So it should
be freed in mwifiex_del_virtual_intf().
This will make sure that wdev will be freed when user deletes an
interface and also in unload path.
2) "priv->netdev->ieee80211_ptr" should also be cleared in
mwifiex_del_virtual_intf.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/mwifiex/cfg80211.c | 3 +++
drivers/net/wireless/mwifiex/main.c | 6 ------
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index ccc9c08..aeaea0e 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -2310,7 +2310,10 @@ int mwifiex_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev)
unregister_netdevice(wdev->netdev);
/* Clear the priv in adapter */
+ priv->netdev->ieee80211_ptr = NULL;
priv->netdev = NULL;
+ kfree(wdev);
+ priv->wdev = NULL;
priv->media_connected = false;
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 42d9a68..78e8a66 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -998,12 +998,6 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter, struct semaphore *sem)
wiphy_unregister(priv->wdev->wiphy);
wiphy_free(priv->wdev->wiphy);
- for (i = 0; i < adapter->priv_num; i++) {
- priv = adapter->priv[i];
- if (priv)
- kfree(priv->wdev);
- }
-
mwifiex_terminate_workqueue(adapter);
/* Unregister device */
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3.13 5/6] mwifiex: fix memory leak issue for sdio and pcie cards
2013-11-15 3:10 [PATCH 3.13 1/6] mwifiex: use return value of mwifiex_add_virtual_intf() correctly Bing Zhao
` (2 preceding siblings ...)
2013-11-15 3:10 ` [PATCH 3.13 4/6] mwifiex: fix potential mem leak in .del_virtual_intf Bing Zhao
@ 2013-11-15 3:10 ` Bing Zhao
2013-11-15 3:10 ` [PATCH 3.13 6/6] mwifiex: fix issues in driver unload path for USB chipsets Bing Zhao
4 siblings, 0 replies; 6+ messages in thread
From: Bing Zhao @ 2013-11-15 3:10 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, Amitkumar Karwar, Ujjal Roy, Avinash Patil,
Nishant Sarmukadam, Frank Huang, Bing Zhao
From: Amitkumar Karwar <akarwar@marvell.com>
When driver is failed to load, card pointer doesn't get
freed. We will free it in cleanup handler which is called
in failure as well as unload path.
Also, update drvdata in init/cleanup handlers instead of
register/unregister handlers.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/mwifiex/pcie.c | 2 +-
drivers/net/wireless/mwifiex/sdio.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c
index 33fa943..03688aa 100644
--- a/drivers/net/wireless/mwifiex/pcie.c
+++ b/drivers/net/wireless/mwifiex/pcie.c
@@ -232,7 +232,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
}
mwifiex_remove_card(card->adapter, &add_remove_card_sem);
- kfree(card);
}
static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
@@ -2313,6 +2312,7 @@ static void mwifiex_pcie_cleanup(struct mwifiex_adapter *adapter)
pci_release_region(pdev, 0);
pci_set_drvdata(pdev, NULL);
}
+ kfree(card);
}
/*
diff --git a/drivers/net/wireless/mwifiex/sdio.c b/drivers/net/wireless/mwifiex/sdio.c
index 9bf8898..b44a315 100644
--- a/drivers/net/wireless/mwifiex/sdio.c
+++ b/drivers/net/wireless/mwifiex/sdio.c
@@ -196,7 +196,6 @@ mwifiex_sdio_remove(struct sdio_func *func)
}
mwifiex_remove_card(card->adapter, &add_remove_card_sem);
- kfree(card);
}
/*
@@ -1745,7 +1744,6 @@ mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
sdio_claim_host(card->func);
sdio_disable_func(card->func);
sdio_release_host(card->func);
- sdio_set_drvdata(card->func, NULL);
}
}
@@ -1773,7 +1771,6 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
return ret;
}
- sdio_set_drvdata(func, card);
adapter->dev = &func->dev;
@@ -1801,6 +1798,8 @@ static int mwifiex_init_sdio(struct mwifiex_adapter *adapter)
int ret;
u8 sdio_ireg;
+ sdio_set_drvdata(card->func, card);
+
/*
* Read the HOST_INT_STATUS_REG for ACK the first interrupt got
* from the bootloader. If we don't do this we get a interrupt
@@ -1883,6 +1882,8 @@ static void mwifiex_cleanup_sdio(struct mwifiex_adapter *adapter)
kfree(card->mpa_rx.len_arr);
kfree(card->mpa_tx.buf);
kfree(card->mpa_rx.buf);
+ sdio_set_drvdata(card->func, NULL);
+ kfree(card);
}
/*
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3.13 6/6] mwifiex: fix issues in driver unload path for USB chipsets
2013-11-15 3:10 [PATCH 3.13 1/6] mwifiex: use return value of mwifiex_add_virtual_intf() correctly Bing Zhao
` (3 preceding siblings ...)
2013-11-15 3:10 ` [PATCH 3.13 5/6] mwifiex: fix memory leak issue for sdio and pcie cards Bing Zhao
@ 2013-11-15 3:10 ` Bing Zhao
4 siblings, 0 replies; 6+ messages in thread
From: Bing Zhao @ 2013-11-15 3:10 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, Amitkumar Karwar, Ujjal Roy, Avinash Patil,
Nishant Sarmukadam, Frank Huang, Bing Zhao
From: Ujjal Roy <royujjal@gmail.com>
1) After driver load failure, clear 'card->adapter' instead of
card pointer so that card specific cleanup is performed later
when user unloads the driver.
2) Clear usb_card pointer in disconnect handler to avoid invalid
memory access when user unloads the driver after removing the
card.
Signed-off-by: Ujjal Roy <royujjal@gmail.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/mwifiex/usb.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 1c70b8d..edf5b7a 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -350,7 +350,6 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
card->udev = udev;
card->intf = intf;
- usb_card = card;
pr_debug("info: bcdUSB=%#x Device Class=%#x SubClass=%#x Protocol=%#x\n",
udev->descriptor.bcdUSB, udev->descriptor.bDeviceClass,
@@ -525,25 +524,28 @@ static int mwifiex_usb_resume(struct usb_interface *intf)
static void mwifiex_usb_disconnect(struct usb_interface *intf)
{
struct usb_card_rec *card = usb_get_intfdata(intf);
- struct mwifiex_adapter *adapter;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card) {
+ pr_err("%s: card is NULL\n", __func__);
return;
}
- adapter = card->adapter;
- if (!adapter->priv_num)
- return;
-
mwifiex_usb_free(card);
- dev_dbg(adapter->dev, "%s: removing card\n", __func__);
- mwifiex_remove_card(adapter, &add_remove_card_sem);
+ if (card->adapter) {
+ struct mwifiex_adapter *adapter = card->adapter;
+
+ if (!adapter->priv_num)
+ return;
+
+ dev_dbg(adapter->dev, "%s: removing card\n", __func__);
+ mwifiex_remove_card(adapter, &add_remove_card_sem);
+ }
usb_set_intfdata(intf, NULL);
usb_put_dev(interface_to_usbdev(intf));
kfree(card);
+ usb_card = NULL;
return;
}
@@ -754,6 +756,7 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
card->adapter = adapter;
adapter->dev = &card->udev->dev;
strcpy(adapter->fw_name, USB8797_DEFAULT_FW_NAME);
+ usb_card = card;
return 0;
}
@@ -762,7 +765,7 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
{
struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
- usb_set_intfdata(card->intf, NULL);
+ card->adapter = NULL;
}
static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
@@ -1004,7 +1007,7 @@ static void mwifiex_usb_cleanup_module(void)
if (!down_interruptible(&add_remove_card_sem))
up(&add_remove_card_sem);
- if (usb_card) {
+ if (usb_card && usb_card->adapter) {
struct mwifiex_adapter *adapter = usb_card->adapter;
int i;
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread