From: Arend van Spriel <arend@broadcom.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless <linux-wireless@vger.kernel.org>,
Hante Meuleman <meuleman@broadcom.com>,
Arend van Spriel <arend@broadcom.com>
Subject: [PATCH 02/11] brcmfmac: Fix race condition between USB probe/load and disconnect.
Date: Thu, 8 Oct 2015 20:33:12 +0200 [thread overview]
Message-ID: <1444329201-3696-3-git-send-email-arend@broadcom.com> (raw)
In-Reply-To: <1444329201-3696-1-git-send-email-arend@broadcom.com>
From: Hante Meuleman <meuleman@broadcom.com>
When a USB device gets disconnected due to for example removal
then it is possible that it is still in the loading phase due to
the asynchronous load routines. These routines can then possible
access memory which has been freed. Fix this by mutex locking the
device init phase.
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
drivers/net/wireless/brcm80211/brcmfmac/usb.c | 29 ++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index daba86d..689e64d 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -144,6 +144,7 @@ struct brcmf_usbdev_info {
struct usb_device *usbdev;
struct device *dev;
+ struct mutex dev_init_lock;
int ctl_in_pipe, ctl_out_pipe;
struct urb *ctl_urb; /* URB for control endpoint */
@@ -1204,6 +1205,8 @@ static void brcmf_usb_probe_phase2(struct device *dev,
int ret;
brcmf_dbg(USB, "Start fw downloading\n");
+
+ devinfo = bus->bus_priv.usb->devinfo;
ret = check_file(fw->data);
if (ret < 0) {
brcmf_err("invalid firmware\n");
@@ -1211,7 +1214,6 @@ static void brcmf_usb_probe_phase2(struct device *dev,
goto error;
}
- devinfo = bus->bus_priv.usb->devinfo;
devinfo->image = fw->data;
devinfo->image_len = fw->size;
@@ -1224,9 +1226,11 @@ static void brcmf_usb_probe_phase2(struct device *dev,
if (ret)
goto error;
+ mutex_unlock(&devinfo->dev_init_lock);
return;
error:
brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), ret);
+ mutex_unlock(&devinfo->dev_init_lock);
device_release_driver(dev);
}
@@ -1264,6 +1268,7 @@ static int brcmf_usb_probe_cb(struct brcmf_usbdev_info *devinfo)
if (ret)
goto fail;
/* we are done */
+ mutex_unlock(&devinfo->dev_init_lock);
return 0;
}
bus->chip = bus_pub->devid;
@@ -1317,6 +1322,12 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
devinfo->usbdev = usb;
devinfo->dev = &usb->dev;
+ /* Take an init lock, to protect for disconnect while still loading.
+ * Necessary because of the asynchronous firmware load construction
+ */
+ mutex_init(&devinfo->dev_init_lock);
+ mutex_lock(&devinfo->dev_init_lock);
+
usb_set_intfdata(intf, devinfo);
/* Check that the device supports only one configuration */
@@ -1391,6 +1402,7 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
return 0;
fail:
+ mutex_unlock(&devinfo->dev_init_lock);
kfree(devinfo);
usb_set_intfdata(intf, NULL);
return ret;
@@ -1403,8 +1415,19 @@ brcmf_usb_disconnect(struct usb_interface *intf)
brcmf_dbg(USB, "Enter\n");
devinfo = (struct brcmf_usbdev_info *)usb_get_intfdata(intf);
- brcmf_usb_disconnect_cb(devinfo);
- kfree(devinfo);
+
+ if (devinfo) {
+ mutex_lock(&devinfo->dev_init_lock);
+ /* Make sure that devinfo still exists. Firmware probe routines
+ * may have released the device and cleared the intfdata.
+ */
+ if (!usb_get_intfdata(intf))
+ goto done;
+
+ brcmf_usb_disconnect_cb(devinfo);
+ kfree(devinfo);
+ }
+done:
brcmf_dbg(USB, "Exit\n");
}
--
1.9.1
next prev parent reply other threads:[~2015-10-08 18:33 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-08 18:33 [PATCH 00/11] brcmfmac: cfg80211 callbacks and coredump Arend van Spriel
2015-10-08 18:33 ` [PATCH 01/11] brcmfmac: expose device memory to devcoredump subsystem Arend van Spriel
2015-10-08 18:59 ` kbuild test robot
2015-10-12 8:59 ` Kalle Valo
2015-10-12 20:05 ` Arend van Spriel
2015-10-14 11:01 ` Kalle Valo
2015-10-14 13:36 ` Arend van Spriel
2015-10-15 10:06 ` Kalle Valo
2015-10-21 7:57 ` [01/11] " Kalle Valo
2015-10-08 18:33 ` Arend van Spriel [this message]
2015-10-08 18:33 ` [PATCH 03/11] brcmfmac: rename firmware_path to alternative_fw_path Arend van Spriel
2015-10-08 18:33 ` [PATCH 04/11] brcmfmac: remove conversational comment Arend van Spriel
2015-10-08 18:33 ` [PATCH 05/11] brcmfmac: Rework p2p attach, use single method for p2p dev creation Arend van Spriel
2015-10-08 18:33 ` [PATCH 06/11] brcmfmac: Fix station info rate information Arend van Spriel
2015-10-08 18:33 ` [PATCH 07/11] brcmfmac: Add RSSI information to get_station Arend van Spriel
2015-10-08 18:33 ` [PATCH 08/11] brcmfmac: Add dump_station support to cfg80221 ops Arend van Spriel
2015-10-08 18:33 ` [PATCH 09/11] brcmfmac: Move brcmf_c_preinit_dcmds prototype to correct file Arend van Spriel
2015-10-08 18:33 ` [PATCH 10/11] brcmfmac: Remove unused state AP creating Arend van Spriel
2015-10-08 18:33 ` [PATCH 11/11] brcmfmac: Properly set carrier state of netdev Arend van Spriel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1444329201-3696-3-git-send-email-arend@broadcom.com \
--to=arend@broadcom.com \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
--cc=meuleman@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).