Stable Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Alan Stern <stern@rowland.harvard.edu>,
	Christian Lamparter <chunkeey@gmail.com>,
	Kalle Valo <kvalo@codeaurora.org>
Subject: [PATCH 5.2 50/61] carl9170: fix misuse of device driver API
Date: Fri, 12 Jul 2019 14:20:03 +0200
Message-ID: <20190712121623.365676357@linuxfoundation.org> (raw)
In-Reply-To: <20190712121620.632595223@linuxfoundation.org>

From: Christian Lamparter <chunkeey@gmail.com>

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: <stable@vger.kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 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



  parent reply index

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 12:19 [PATCH 5.2 00/61] 5.2.1-stable review Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 01/61] crypto: talitos - fix hash on SEC1 Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 02/61] crypto: lrw - use correct alignmask Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 03/61] crypto: talitos - rename alternative AEAD algos Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 04/61] fscrypt: dont set policy for a dead directory Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 05/61] udf: Fix incorrect final NOT_ALLOCATED (hole) extent length Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 06/61] media: stv0297: fix frequency range limit Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 07/61] ALSA: usb-audio: Fix parse of UAC2 Extension Units Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 08/61] ALSA: hda/realtek - Headphone Mic cant record after S3 Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 09/61] tpm: Actually fail on TPM errors during "get random" Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 10/61] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 11/61] block: fix .bi_size overflow Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 12/61] block, bfq: NULL out the bic when its no longer valid Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 13/61] perf intel-pt: Fix itrace defaults for perf script Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 14/61] perf auxtrace: " Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 15/61] perf intel-pt: Fix itrace defaults for perf script intel-pt documentation Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 16/61] perf pmu: Fix uncore PMU alias list for ARM64 Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 17/61] perf thread-stack: Fix thread stack return from kernel for kernel-only case Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 18/61] perf header: Assign proper ff->ph in perf_event__synthesize_features() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 19/61] x86/ptrace: Fix possible spectre-v1 in ptrace_get_debugreg() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 20/61] x86/tls: Fix possible spectre-v1 in do_get_thread_area() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 21/61] Documentation: Add section about CPU vulnerabilities for Spectre Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 22/61] Documentation/admin: Remove the vsyscall=native documentation Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 23/61] mwifiex: Dont abort on small, spec-compliant vendor IEs Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 24/61] USB: serial: ftdi_sio: add ID for isodebug v1 Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 25/61] USB: serial: option: add support for GosunCn ME3630 RNDIS mode Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 26/61] Revert "serial: 8250: Dont service RX FIFO if interrupts are disabled" Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 27/61] p54usb: Fix race between disconnect and firmware loading Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 28/61] usb: gadget: f_fs: data_len used before properly set Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 29/61] usb: gadget: ether: Fix race between gether_disconnect and rx_submit Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 30/61] usb: dwc2: use a longer AHB idle timeout in dwc2_core_reset() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 31/61] usb: renesas_usbhs: add a workaround for a race condition of workqueue Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 32/61] drivers/usb/typec/tps6598x.c: fix portinfo width Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 33/61] drivers/usb/typec/tps6598x.c: fix 4CC cmd write Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 34/61] p54: fix crash during initialization Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 35/61] staging: comedi: dt282x: fix a null pointer deref on interrupt Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 36/61] staging: wilc1000: fix error path cleanup in wilc_wlan_initialize() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 37/61] staging: bcm2835-camera: Restore return behavior of ctrl_set_bitrate() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 38/61] staging: comedi: amplc_pci230: fix null pointer deref on interrupt Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 39/61] staging: mt7621-pci: fix PCIE_FTS_NUM_LO macro Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 40/61] HID: Add another Primax PIXART OEM mouse quirk Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 41/61] lkdtm: support llvm-objcopy Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 42/61] binder: fix memory leak in error path Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 43/61] binder: return errors from buffer copy functions Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 44/61] iio: adc: stm32-adc: add missing vdda-supply Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 45/61] coresight: Potential uninitialized variable in probe() Greg Kroah-Hartman
2019-07-12 12:19 ` [PATCH 5.2 46/61] coresight: etb10: Do not call smp_processor_id from preemptible Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 47/61] coresight: tmc-etr: Do not call smp_processor_id() " Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 48/61] coresight: tmc-etr: alloc_perf_buf: Do not call smp_processor_id " Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 49/61] coresight: tmc-etf: " Greg Kroah-Hartman
2019-07-12 12:20 ` Greg Kroah-Hartman [this message]
2019-07-12 12:20 ` [PATCH 5.2 51/61] Revert "x86/build: Move _etext to actual end of .text" Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 52/61] VMCI: Fix integer overflow in VMCI handle arrays Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 53/61] staging: vchiq_2835_arm: revert "quit using custom down_interruptible()" Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 54/61] staging: vchiq: make wait events interruptible Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 55/61] staging: vchiq: revert "switch to wait_for_completion_killable" Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 56/61] staging: fsl-dpaa2/ethsw: fix memory leak of switchdev_work Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 57/61] staging: bcm2835-camera: Replace spinlock protecting context_map with mutex Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 58/61] staging: bcm2835-camera: Ensure all buffers are returned on disable Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 59/61] staging: bcm2835-camera: Remove check of the number of buffers supplied Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 60/61] staging: bcm2835-camera: Handle empty EOS buffers whilst streaming Greg Kroah-Hartman
2019-07-12 12:20 ` [PATCH 5.2 61/61] staging: rtl8712: reduce stack usage, again Greg Kroah-Hartman
2019-07-12 13:36 ` [PATCH 5.2 00/61] 5.2.1-stable review Jon Hunter
2019-07-12 15:31   ` Greg Kroah-Hartman
2019-07-12 16:10     ` Jon Hunter
2019-07-12 16:51       ` Greg Kroah-Hartman
2019-07-12 17:00 ` Major Hayden
2019-07-12 17:17   ` Dan Rue
2019-07-12 22:07 ` shuah
2019-07-13  8:23   ` Greg Kroah-Hartman
2019-07-13  3:04 ` Naresh Kamboju
2019-07-13 14:32   ` Greg Kroah-Hartman
2019-07-13 10:15 ` Shreeya Patel
2019-07-13 15:21   ` Greg Kroah-Hartman
2019-07-13 20:37 ` Luke Nowakowski-Krijger
2019-07-14  6:02   ` Greg Kroah-Hartman
2019-07-13 22:04 ` Guenter Roeck
2019-07-14  6:02   ` Greg Kroah-Hartman
2019-07-14  5:35 ` Kelsey Skunberg
2019-07-14  6:02   ` Greg Kroah-Hartman

Reply instructions:

You may reply publically 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=20190712121623.365676357@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=chunkeey@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org stable@archiver.kernel.org
	public-inbox-index stable


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox