From patchwork Thu Oct 8 18:33:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arend van Spriel X-Patchwork-Id: 7354831 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CE65B9F4DC for ; Thu, 8 Oct 2015 18:33:40 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F29B9207CE for ; Thu, 8 Oct 2015 18:33:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EA5D5207CD for ; Thu, 8 Oct 2015 18:33:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755903AbbJHSd3 (ORCPT ); Thu, 8 Oct 2015 14:33:29 -0400 Received: from mail-gw1-out.broadcom.com ([216.31.210.62]:54408 "EHLO mail-gw1-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755424AbbJHSdZ (ORCPT ); Thu, 8 Oct 2015 14:33:25 -0400 X-IronPort-AV: E=Sophos;i="5.17,655,1437462000"; d="scan'208";a="77224830" Received: from irvexchcas08.broadcom.com (HELO IRVEXCHCAS08.corp.ad.broadcom.com) ([10.9.208.57]) by mail-gw1-out.broadcom.com with ESMTP; 08 Oct 2015 13:23:29 -0700 Received: from IRVEXCHSMTP3.corp.ad.broadcom.com (10.9.207.53) by IRVEXCHCAS08.corp.ad.broadcom.com (10.9.208.57) with Microsoft SMTP Server (TLS) id 14.3.235.1; Thu, 8 Oct 2015 11:33:24 -0700 Received: from mail-irva-13.broadcom.com (10.10.10.20) by IRVEXCHSMTP3.corp.ad.broadcom.com (10.9.207.53) with Microsoft SMTP Server id 14.3.235.1; Thu, 8 Oct 2015 11:33:23 -0700 Received: from bld-bun-01.bun.broadcom.com (unknown [10.176.128.83]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 143B040FE8; Thu, 8 Oct 2015 11:30:32 -0700 (PDT) Received: by bld-bun-01.bun.broadcom.com (Postfix, from userid 25152) id 03787B006CE; Thu, 8 Oct 2015 20:33:23 +0200 (CEST) From: Arend van Spriel To: Kalle Valo CC: linux-wireless , Hante Meuleman , Arend van Spriel Subject: [PATCH 02/11] brcmfmac: Fix race condition between USB probe/load and disconnect. Date: Thu, 8 Oct 2015 20:33:12 +0200 Message-ID: <1444329201-3696-3-git-send-email-arend@broadcom.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1444329201-3696-1-git-send-email-arend@broadcom.com> References: <1444329201-3696-1-git-send-email-arend@broadcom.com> MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Hante Meuleman 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 Reviewed-by: Pieter-Paul Giesberts Signed-off-by: Hante Meuleman Signed-off-by: Arend van Spriel --- 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"); }