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,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>
Subject: [PATCH 4.14 34/39] HID: hiddev: fix mess in hiddev_open()
Date: Tue, 14 Jan 2020 11:02:08 +0100
Message-ID: <20200114094346.202352828@linuxfoundation.org> (raw)
In-Reply-To: <20200114094336.210038037@linuxfoundation.org>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

commit 18a1b06e5b91d47dc86c0a66a762646ea7c5d141 upstream.

The open method of hiddev handler fails to bring the device out of
autosuspend state as was promised in 0361a28d3f9a, as it actually has 2
blocks that try to start the transport (call hid_hw_open()) with both
being guarded by the "open" counter, so the 2nd block is never executed as
the first block increments the counter so it is never at 0 when we check
it for the second block.

Additionally hiddev_open() was leaving counter incremented on errors,
causing the device to never be reopened properly if there was ever an
error.

Let's fix all of this by factoring out code that creates client structure
and powers up the device into a separate function that is being called
from usbhid_open() with the "existancelock" being held.

Fixes: 0361a28d3f9a ("HID: autosuspend support for USB HID")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/hid/usbhid/hiddev.c |   97 +++++++++++++++++++-------------------------
 1 file changed, 42 insertions(+), 55 deletions(-)

--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -254,12 +254,51 @@ static int hiddev_release(struct inode *
 	return 0;
 }
 
+static int __hiddev_open(struct hiddev *hiddev, struct file *file)
+{
+	struct hiddev_list *list;
+	int error;
+
+	lockdep_assert_held(&hiddev->existancelock);
+
+	list = vzalloc(sizeof(*list));
+	if (!list)
+		return -ENOMEM;
+
+	mutex_init(&list->thread_lock);
+	list->hiddev = hiddev;
+
+	if (!hiddev->open++) {
+		error = hid_hw_power(hiddev->hid, PM_HINT_FULLON);
+		if (error < 0)
+			goto err_drop_count;
+
+		error = hid_hw_open(hiddev->hid);
+		if (error < 0)
+			goto err_normal_power;
+	}
+
+	spin_lock_irq(&hiddev->list_lock);
+	list_add_tail(&list->node, &hiddev->list);
+	spin_unlock_irq(&hiddev->list_lock);
+
+	file->private_data = list;
+
+	return 0;
+
+err_normal_power:
+	hid_hw_power(hiddev->hid, PM_HINT_NORMAL);
+err_drop_count:
+	hiddev->open--;
+	vfree(list);
+	return error;
+}
+
 /*
  * open file op
  */
 static int hiddev_open(struct inode *inode, struct file *file)
 {
-	struct hiddev_list *list;
 	struct usb_interface *intf;
 	struct hid_device *hid;
 	struct hiddev *hiddev;
@@ -268,66 +307,14 @@ static int hiddev_open(struct inode *ino
 	intf = usbhid_find_interface(iminor(inode));
 	if (!intf)
 		return -ENODEV;
+
 	hid = usb_get_intfdata(intf);
 	hiddev = hid->hiddev;
 
-	if (!(list = vzalloc(sizeof(struct hiddev_list))))
-		return -ENOMEM;
-	mutex_init(&list->thread_lock);
-	list->hiddev = hiddev;
-	file->private_data = list;
-
-	/*
-	 * no need for locking because the USB major number
-	 * is shared which usbcore guards against disconnect
-	 */
-	if (list->hiddev->exist) {
-		if (!list->hiddev->open++) {
-			res = hid_hw_open(hiddev->hid);
-			if (res < 0)
-				goto bail;
-		}
-	} else {
-		res = -ENODEV;
-		goto bail;
-	}
-
-	spin_lock_irq(&list->hiddev->list_lock);
-	list_add_tail(&list->node, &hiddev->list);
-	spin_unlock_irq(&list->hiddev->list_lock);
-
 	mutex_lock(&hiddev->existancelock);
-	/*
-	 * recheck exist with existance lock held to
-	 * avoid opening a disconnected device
-	 */
-	if (!list->hiddev->exist) {
-		res = -ENODEV;
-		goto bail_unlock;
-	}
-	if (!list->hiddev->open++)
-		if (list->hiddev->exist) {
-			struct hid_device *hid = hiddev->hid;
-			res = hid_hw_power(hid, PM_HINT_FULLON);
-			if (res < 0)
-				goto bail_unlock;
-			res = hid_hw_open(hid);
-			if (res < 0)
-				goto bail_normal_power;
-		}
-	mutex_unlock(&hiddev->existancelock);
-	return 0;
-bail_normal_power:
-	hid_hw_power(hid, PM_HINT_NORMAL);
-bail_unlock:
+	res = hiddev->exist ? __hiddev_open(hiddev, file) : -ENODEV;
 	mutex_unlock(&hiddev->existancelock);
 
-	spin_lock_irq(&list->hiddev->list_lock);
-	list_del(&list->node);
-	spin_unlock_irq(&list->hiddev->list_lock);
-bail:
-	file->private_data = NULL;
-	vfree(list);
 	return res;
 }
 



  parent reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-14 10:01 [PATCH 4.14 00/39] 4.14.165-stable review Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 01/39] chardev: Avoid potential use-after-free in chrdev_open() Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 02/39] usb: chipidea: host: Disable port power only if previously enabled Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 03/39] ALSA: usb-audio: Apply the sample rate quirk for Bose Companion 5 Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 04/39] ALSA: hda/realtek - Add new codec supported for ALCS1200A Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 05/39] ALSA: hda/realtek - Set EAPD control to default for ALC222 Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 06/39] kernel/trace: Fix do not unregister tracepoints when register sched_migrate_task fail Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 07/39] tracing: Have stack tracer compile when MCOUNT_INSN_SIZE is not defined Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 08/39] HID: Fix slab-out-of-bounds read in hid_field_extract Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 09/39] HID: uhid: Fix returning EPOLLOUT from uhid_char_poll Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 10/39] can: gs_usb: gs_usb_probe(): use descriptors of current altsetting Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 11/39] can: mscan: mscan_rx_poll(): fix rx path lockup when returning from polling to irq mode Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 12/39] can: can_dropped_invalid_skb(): ensure an initialized headroom in outgoing CAN sk_buffs Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 13/39] gpiolib: acpi: Turn dmi_system_id table into a generic quirk table Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 14/39] gpiolib: acpi: Add honor_wakeup module-option + quirk mechanism Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 15/39] staging: vt6656: set usb_set_intfdata on driver fail Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 16/39] USB: serial: option: add ZLP support for 0x1bc7/0x9010 Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 17/39] usb: musb: fix idling for suspend after disconnect interrupt Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 18/39] usb: musb: Disable pullup at init Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 19/39] usb: musb: dma: Correct parameter passed to IRQ handler Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 20/39] staging: comedi: adv_pci1710: fix AI channels 16-31 for PCI-1713 Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 21/39] HID: hid-input: clear unmapped usages Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 22/39] Input: add safety guards to input_set_keycode() Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 23/39] drm/fb-helper: Round up bits_per_pixel if possible Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 24/39] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ Greg Kroah-Hartman
2020-01-14 10:01 ` [PATCH 4.14 25/39] staging: rtl8188eu: Add device code for TP-Link TL-WN727N v5.21 Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 26/39] tty: link tty and port before configuring it as console Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 27/39] tty: always relink the port Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 28/39] mwifiex: fix possible heap overflow in mwifiex_process_country_ie() Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 29/39] mwifiex: pcie: Fix memory leak in mwifiex_pcie_alloc_cmdrsp_buf Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 30/39] scsi: bfa: release allocated memory in case of error Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 31/39] rtl8xxxu: prevent leaking urb Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 32/39] ath10k: fix memory leak Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 33/39] arm64: cpufeature: Avoid warnings due to unused symbols Greg Kroah-Hartman
2020-01-14 10:02 ` Greg Kroah-Hartman [this message]
2020-01-14 10:02 ` [PATCH 4.14 35/39] USB: Fix: Dont skip endpoint descriptors with maxpacket=0 Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 36/39] phy: cpcap-usb: Fix error path when no host driver is loaded Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 37/39] phy: cpcap-usb: Fix flakey host idling and enumerating of devices Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 38/39] netfilter: arp_tables: init netns pointer in xt_tgchk_param struct Greg Kroah-Hartman
2020-01-14 10:02 ` [PATCH 4.14 39/39] netfilter: ipset: avoid null deref when IPSET_ATTR_LINENO is present Greg Kroah-Hartman
2020-01-14 15:02 ` [PATCH 4.14 00/39] 4.14.165-stable review Jon Hunter
2020-01-14 18:15 ` Guenter Roeck
2020-01-14 20:27 ` shuah
2020-01-15  2:08 ` Daniel Díaz

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=20200114094346.202352828@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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
	public-inbox-index stable

Example config snippet for mirrors

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