Received: by 2002:a25:ad19:0:0:0:0:0 with SMTP id y25csp835177ybi; Fri, 12 Jul 2019 05:26:42 -0700 (PDT) X-Google-Smtp-Source: APXvYqxVzGW/P5b2hcOVwiz1kxPpFCT3AJP/Bvpkfye+tFHrLxWhakF3/VS26lbSubelE0i4DRWb X-Received: by 2002:a17:902:a504:: with SMTP id s4mr11066061plq.117.1562934402252; Fri, 12 Jul 2019 05:26:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1562934402; cv=none; d=google.com; s=arc-20160816; b=ZRq8CxVcHkV2+MF1mIHu0OpkV8k9vTsAQaVPhSVTtm5q6aaWAvgiGWYQt+BBt5uwyR +yfQciimaFyvBX+PL0VEOWl2rEwAYgQIYtpToNIVwb/LcPPmiUxJ+4Wy2x6IFWaIl0fU UOedGcG0JxH7Y1QXocrAF0RRxHa5MbgQOnfYut9gCxAogtqw2PODhllylSmcL9RXOw3C c/i+ahBHLcCv51D+kQMJLAzr7Dp85qDcZUseePi6bd3bkfbgukWw9fpuU1KNKG1gPCJn 9qzJ7fYtgODqMS92zP14L7rkGjfi6OxVUnakKUl+ksld2ejmFTAtF4kk9RfBy/TpyJNL 3ghw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=gbcjQjlt96g2HauZouu9LaWuGU6932pz4TzkfPsZNyc=; b=jAw8eOoRKJVSE5+Z/47EjPxFFEpmAcPE7NpEns66zezU5rArefS3Q5krP3n/U54xea MWrF9wEvPrVu495a8Kb+2vIBKH3JBYuCqAwwnFZWp/to14wwEBUbFRTPjAPqIJlZ6O2Q K2o/m/h2tQIdZcALvNBLtAHvgV3oY7MrHre60are478N6uqfW6csRcRJQDzT6BrJkf97 YNYmB7lEmthRSwZkQgMkl3BoQh03+3jYVdJPvMdhnGjAyfmWmZPiIa7kXPz2qwWI9py8 TCFnq7dSfoUNGLiINLOdTXSZQWYzS/K4VY3688mFsmEGQA9p+xnyFGmlkGjaymHFbLpk 3omA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=wuldkr+k; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q3si12066515pjb.1.2019.07.12.05.26.26; Fri, 12 Jul 2019 05:26:42 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=wuldkr+k; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726582AbfGLMYS (ORCPT + 99 others); Fri, 12 Jul 2019 08:24:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:60504 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728030AbfGLMYH (ORCPT ); Fri, 12 Jul 2019 08:24:07 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id E7152216C8; Fri, 12 Jul 2019 12:24:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1562934246; bh=yoCpDKEXgc/jPOEZGenf4VgfdDgWReN7+VHm3FbOkeQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wuldkr+kidNEYC5lYHND0scGBghE21GGa49phyxA9njuZBMZ6JXSpzUKj4ka4deYq Ayx6te3dig+CsGjALmnLI6G9J0JDIN1EXrTfjBsIfUj1Nr7mK/Cy3IzGgerpnodx1u HfH5AIkVEkvHVr21V7q47MzOSm2CIQDLMT5lWrcU= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Alan Stern , Christian Lamparter , Kalle Valo Subject: [PATCH 4.19 83/91] carl9170: fix misuse of device driver API Date: Fri, 12 Jul 2019 14:19:26 +0200 Message-Id: <20190712121626.215988016@linuxfoundation.org> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190712121621.422224300@linuxfoundation.org> References: <20190712121621.422224300@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Christian Lamparter commit feb09b2933275a70917a869989ea2823e7356be8 upstream. This patch follows Alan Stern's recent patch: "p54: Fix race between disconnect and firmware loading" that overhauled carl9170 buggy firmware loading and driver unbinding procedures. Since the carl9170 code was adapted from p54 it uses the same functions and is likely to have the same problem, but it's just that the syzbot hasn't reproduce them (yet). a summary from the changes (copied from the p54 patch): * Call usb_driver_release_interface() rather than device_release_driver(). * Lock udev (the interface's parent) before unbinding the driver instead of locking udev->parent. * During the firmware loading process, take a reference to the USB interface instead of the USB device. * Don't take an unnecessary reference to the device during probe (and then don't drop it during disconnect). and * Make sure to prevent use-after-free bugs by explicitly setting the driver context to NULL after signaling the completion. Cc: Cc: Alan Stern Signed-off-by: Christian Lamparter Acked-by: Alan Stern Signed-off-by: Kalle Valo Signed-off-by: Greg Kroah-Hartman --- drivers/net/wireless/ath/carl9170/usb.c | 39 +++++++++++++------------------- 1 file changed, 17 insertions(+), 22 deletions(-) --- a/drivers/net/wireless/ath/carl9170/usb.c +++ b/drivers/net/wireless/ath/carl9170/usb.c @@ -128,6 +128,8 @@ static const struct usb_device_id carl91 }; MODULE_DEVICE_TABLE(usb, carl9170_usb_ids); +static struct usb_driver carl9170_driver; + static void carl9170_usb_submit_data_urb(struct ar9170 *ar) { struct urb *urb; @@ -966,32 +968,28 @@ err_out: static void carl9170_usb_firmware_failed(struct ar9170 *ar) { - struct device *parent = ar->udev->dev.parent; - struct usb_device *udev; - - /* - * Store a copy of the usb_device pointer locally. - * This is because device_release_driver initiates - * carl9170_usb_disconnect, which in turn frees our - * driver context (ar). + /* Store a copies of the usb_interface and usb_device pointer locally. + * This is because release_driver initiates carl9170_usb_disconnect, + * which in turn frees our driver context (ar). */ - udev = ar->udev; + struct usb_interface *intf = ar->intf; + struct usb_device *udev = ar->udev; complete(&ar->fw_load_wait); + /* at this point 'ar' could be already freed. Don't use it anymore */ + ar = NULL; /* unbind anything failed */ - if (parent) - device_lock(parent); - - device_release_driver(&udev->dev); - if (parent) - device_unlock(parent); + usb_lock_device(udev); + usb_driver_release_interface(&carl9170_driver, intf); + usb_unlock_device(udev); - usb_put_dev(udev); + usb_put_intf(intf); } static void carl9170_usb_firmware_finish(struct ar9170 *ar) { + struct usb_interface *intf = ar->intf; int err; err = carl9170_parse_firmware(ar); @@ -1009,7 +1007,7 @@ static void carl9170_usb_firmware_finish goto err_unrx; complete(&ar->fw_load_wait); - usb_put_dev(ar->udev); + usb_put_intf(intf); return; err_unrx: @@ -1052,7 +1050,6 @@ static int carl9170_usb_probe(struct usb return PTR_ERR(ar); udev = interface_to_usbdev(intf); - usb_get_dev(udev); ar->udev = udev; ar->intf = intf; ar->features = id->driver_info; @@ -1094,15 +1091,14 @@ static int carl9170_usb_probe(struct usb atomic_set(&ar->rx_anch_urbs, 0); atomic_set(&ar->rx_pool_urbs, 0); - usb_get_dev(ar->udev); + usb_get_intf(intf); carl9170_set_state(ar, CARL9170_STOPPED); err = request_firmware_nowait(THIS_MODULE, 1, CARL9170FW_NAME, &ar->udev->dev, GFP_KERNEL, ar, carl9170_usb_firmware_step2); if (err) { - usb_put_dev(udev); - usb_put_dev(udev); + usb_put_intf(intf); carl9170_free(ar); } return err; @@ -1131,7 +1127,6 @@ static void carl9170_usb_disconnect(stru carl9170_release_firmware(ar); carl9170_free(ar); - usb_put_dev(udev); } #ifdef CONFIG_PM