From: Amitkumar Karwar <[email protected]>
mwifiex_add_virtual_intf() returns ERR_PTR values. So use IS_ERR()
macro instead of checking for NULL pointer.
Reported-by: Ujjal Roy <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
drivers/net/wireless/mwifiex/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 9d7c9d3..7c7da3e 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -418,6 +418,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
struct mwifiex_fw_image fw;
struct semaphore *sem = adapter->card_sem;
bool init_failed = false;
+ struct wireless_dev *wdev;
if (!firmware) {
dev_err(adapter->dev,
@@ -474,8 +475,9 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
rtnl_lock();
/* Create station interface by default */
- if (!mwifiex_add_virtual_intf(adapter->wiphy, "mlan%d",
- NL80211_IFTYPE_STATION, NULL, NULL)) {
+ wdev = mwifiex_add_virtual_intf(adapter->wiphy, "mlan%d",
+ NL80211_IFTYPE_STATION, NULL, NULL);
+ if (IS_ERR(wdev)) {
dev_err(adapter->dev, "cannot create default STA interface\n");
goto err_add_intf;
}
--
1.8.2.3
From: Amitkumar Karwar <[email protected]>
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 <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
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
From: Ujjal Roy <[email protected]>
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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
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
From: Amitkumar Karwar <[email protected]>
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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
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
From: Amitkumar Karwar <[email protected]>
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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
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
From: Amitkumar Karwar <[email protected]>
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 <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
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