diff options
author | Brian Norris <briannorris@chromium.org> | 2016-11-18 17:00:26 +0300 |
---|---|---|
committer | Kalle Valo <kvalo@codeaurora.org> | 2016-11-19 10:18:48 +0300 |
commit | 4a79aa17d53ea8d5fa4acdaed487a786a185936a (patch) | |
tree | 17fac95b50f4108fc9edad96275494b534f1cd42 /drivers/net/wireless/marvell/mwifiex/usb.c | |
parent | 6712076883ca49532b4e15216fb69cde69a5a919 (diff) | |
download | linux-4a79aa17d53ea8d5fa4acdaed487a786a185936a.tar.xz |
mwifiex: resolve races between async FW init (failure) and device removal
It's possible for the FW init sequence to fail, which will trigger a
device cleanup sequence in mwifiex_fw_dpc(). This sequence can race with
device suspend() or remove() (e.g., reboot or unbind), and can trigger
use-after-free issues. Currently, this driver attempts (poorly) to
synchronize remove() using a semaphore, but it doesn't protect some of
the critical sections properly. Particularly, we grab a pointer to the
adapter struct (card->adapter) without checking if it's being freed or
not. We later do a NULL check on the adapter, but that doesn't work if
the adapter was freed.
Also note that the PCIe interface driver doesn't ever set card->adapter
to NULL, so even if we get the synchronization right, we still might try
to redo the cleanup in ->remove(), even if the FW init failure sequence
already did it.
This patch replaces the static semaphore with a per-device completion
struct, and uses that completion to synchronize the remove() thread with
the mwifiex_fw_dpc(). A future patch will utilize this completion to
synchronize the suspend() thread as well.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Diffstat (limited to 'drivers/net/wireless/marvell/mwifiex/usb.c')
-rw-r--r-- | drivers/net/wireless/marvell/mwifiex/usb.c | 23 |
1 files changed, 10 insertions, 13 deletions
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c index 9cc2a0c0b670..63a755c1e38c 100644 --- a/drivers/net/wireless/marvell/mwifiex/usb.c +++ b/drivers/net/wireless/marvell/mwifiex/usb.c @@ -24,7 +24,6 @@ static u8 user_rmmod; static struct mwifiex_if_ops usb_ops; -static struct semaphore add_remove_card_sem; static struct usb_device_id mwifiex_usb_table[] = { /* 8766 */ @@ -386,6 +385,8 @@ static int mwifiex_usb_probe(struct usb_interface *intf, if (!card) return -ENOMEM; + init_completion(&card->fw_done); + id_vendor = le16_to_cpu(udev->descriptor.idVendor); id_product = le16_to_cpu(udev->descriptor.idProduct); bcd_device = le16_to_cpu(udev->descriptor.bcdDevice); @@ -475,7 +476,7 @@ static int mwifiex_usb_probe(struct usb_interface *intf, usb_set_intfdata(intf, card); - ret = mwifiex_add_card(card, &add_remove_card_sem, &usb_ops, + ret = mwifiex_add_card(card, &card->fw_done, &usb_ops, MWIFIEX_USB, &card->udev->dev); if (ret) { pr_err("%s: mwifiex_add_card failed: %d\n", __func__, ret); @@ -601,13 +602,15 @@ 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) { + dev_err(&intf->dev, "%s: card is NULL\n", __func__); return; } + wait_for_completion(&card->fw_done); + adapter = card->adapter; - if (!adapter->priv_num) + if (!adapter || !adapter->priv_num) return; if (user_rmmod && !adapter->mfg_mode) { @@ -627,7 +630,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf) mwifiex_dbg(adapter, FATAL, "%s: removing card\n", __func__); - mwifiex_remove_card(adapter, &add_remove_card_sem); + mwifiex_remove_card(adapter); usb_put_dev(interface_to_usbdev(intf)); } @@ -1200,8 +1203,7 @@ static struct mwifiex_if_ops usb_ops = { /* This function initializes the USB driver module. * - * This initiates the semaphore and registers the device with - * USB bus. + * This registers the device with USB bus. */ static int mwifiex_usb_init_module(void) { @@ -1209,8 +1211,6 @@ static int mwifiex_usb_init_module(void) pr_debug("Marvell USB8797 Driver\n"); - sema_init(&add_remove_card_sem, 1); - ret = usb_register(&mwifiex_usb_driver); if (ret) pr_err("Driver register failed!\n"); @@ -1230,9 +1230,6 @@ static int mwifiex_usb_init_module(void) */ static void mwifiex_usb_cleanup_module(void) { - if (!down_interruptible(&add_remove_card_sem)) - up(&add_remove_card_sem); - /* set the flag as user is removing this module */ user_rmmod = 1; |