linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>,
	Simon Gene Gottlieb <simon@gottliebtfreitag.de>,
	Jiri Kosina <jkosina@suse.cz>, Sasha Levin <sashal@kernel.org>,
	linux-input@vger.kernel.org
Subject: [PATCH AUTOSEL 4.19 029/100] HID: steam: fix deadlock with input devices.
Date: Fri, 18 Oct 2019 18:04:14 -0400	[thread overview]
Message-ID: <20191018220525.9042-29-sashal@kernel.org> (raw)
In-Reply-To: <20191018220525.9042-1-sashal@kernel.org>

From: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>

[ Upstream commit 6b538cc21334b83f09b25dec4aa2d2726bf07ed0 ]

When using this driver with the wireless dongle and some usermode
program that monitors every input device (acpid, for example), while
another usermode client opens and closes the low-level device
repeadedly, the system eventually deadlocks.

The reason is that steam_input_register_device() must not be called with
the mutex held, because the input subsystem has its own synchronization
that clashes with this one: it is possible that steam_input_open() is
called before input_register_device() returns, and since
steam_input_open() needs to lock the mutex, it deadlocks.

However we must hold the mutex when calling any function that sends
commands to the controller. If not, random commands end up falling fail.

Reported-by: Simon Gene Gottlieb <simon@gottliebtfreitag.de>
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
Tested-by: Simon Gene Gottlieb <simon@gottliebtfreitag.de>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-steam.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 8141cadfca0e3..8dae0f9b819e0 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -499,6 +499,7 @@ static void steam_battery_unregister(struct steam_device *steam)
 static int steam_register(struct steam_device *steam)
 {
 	int ret;
+	bool client_opened;
 
 	/*
 	 * This function can be called several times in a row with the
@@ -511,9 +512,11 @@ static int steam_register(struct steam_device *steam)
 		 * Unlikely, but getting the serial could fail, and it is not so
 		 * important, so make up a serial number and go on.
 		 */
+		mutex_lock(&steam->mutex);
 		if (steam_get_serial(steam) < 0)
 			strlcpy(steam->serial_no, "XXXXXXXXXX",
 					sizeof(steam->serial_no));
+		mutex_unlock(&steam->mutex);
 
 		hid_info(steam->hdev, "Steam Controller '%s' connected",
 				steam->serial_no);
@@ -528,13 +531,15 @@ static int steam_register(struct steam_device *steam)
 	}
 
 	mutex_lock(&steam->mutex);
-	if (!steam->client_opened) {
+	client_opened = steam->client_opened;
+	if (!client_opened)
 		steam_set_lizard_mode(steam, lizard_mode);
+	mutex_unlock(&steam->mutex);
+
+	if (!client_opened)
 		ret = steam_input_register(steam);
-	} else {
+	else
 		ret = 0;
-	}
-	mutex_unlock(&steam->mutex);
 
 	return ret;
 }
@@ -630,14 +635,21 @@ static void steam_client_ll_close(struct hid_device *hdev)
 {
 	struct steam_device *steam = hdev->driver_data;
 
+	unsigned long flags;
+	bool connected;
+
+	spin_lock_irqsave(&steam->lock, flags);
+	connected = steam->connected;
+	spin_unlock_irqrestore(&steam->lock, flags);
+
 	mutex_lock(&steam->mutex);
 	steam->client_opened = false;
+	if (connected)
+		steam_set_lizard_mode(steam, lizard_mode);
 	mutex_unlock(&steam->mutex);
 
-	if (steam->connected) {
-		steam_set_lizard_mode(steam, lizard_mode);
+	if (connected)
 		steam_input_register(steam);
-	}
 }
 
 static int steam_client_ll_raw_request(struct hid_device *hdev,
-- 
2.20.1


  parent reply	other threads:[~2019-10-18 22:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20191018220525.9042-1-sashal@kernel.org>
2019-10-18 22:03 ` [PATCH AUTOSEL 4.19 008/100] HID: i2c-hid: add Direkt-Tek DTLAPY133-1 to descriptor override Sasha Levin
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 017/100] HID: i2c-hid: Disable runtime PM for LG touchscreen Sasha Levin
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 018/100] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels Sasha Levin
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 019/100] HID: i2c-hid: Add Odys Winbook 13 to descriptor override Sasha Levin
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 026/100] HID: Add ASUS T100CHI keyboard dock battery quirks Sasha Levin
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 028/100] HID: steam: fix boot loop with bluetooth firmware Sasha Levin
2019-10-18 22:04 ` Sasha Levin [this message]
2019-10-18 22:04 ` [PATCH AUTOSEL 4.19 066/100] HID: hyperv: Use in-place iterator API in the channel callback Sasha Levin

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=20191018220525.9042-29-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigorivascosta@gmail.com \
    --cc=simon@gottliebtfreitag.de \
    --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
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).