From: Lee Jones <lee@kernel.org>
To: lee@kernel.org, Jiri Kosina <jikos@kernel.org>,
Benjamin Tissoires <bentiss@kernel.org>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org, Vicki Pfau <vi@endrift.com>,
Jiri Kosina <jkosina@suse.com>
Subject: [STABLE v6.18 4/4] HID: steam: Clean up locking
Date: Tue, 13 Oct 2026 09:11:35 +0000 [thread overview]
Message-ID: <20261013091138.526840-4-lee@kernel.org> (raw)
In-Reply-To: <20261013091138.526840-1-lee@kernel.org>
From: Vicki Pfau <vi@endrift.com>
[ Upstream commit de1d341b9b3ec9655f31fb8b44f784cca5e670ad ]
This cleans up several issues with locking behavior, including RCU accesses
not being guarded behind a lock.
Signed-off-by: Vicki Pfau <vi@endrift.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
[Lee: Drop report_mutex from steam_get_serial() and
steam_get_attributes() instead of steam_exchange_report_id() and
omit coalesce_rumble_work and Ibex changes as 1d4549fd1115,
fcfa7db89f7a and 0a80b4e8ec6a are not present]
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-steam.c | 103 +++++++++++++++++++++-------------------
1 file changed, 55 insertions(+), 48 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index e2b7df274eb3..84c90091d45f 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -327,6 +327,7 @@ struct steam_device {
spinlock_t lock;
struct hid_device *hdev, *client_hdev;
struct mutex report_mutex;
+ struct mutex registration_mutex;
unsigned long client_opened;
struct input_dev __rcu *input;
struct input_dev __rcu *sensors;
@@ -510,7 +511,6 @@ static int steam_get_serial(struct steam_device *steam)
u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, sizeof(steam->serial_no), ATTRIB_STR_UNIT_SERIAL};
u8 reply[3 + STEAM_SERIAL_LEN + 1];
- guard(mutex)(&steam->report_mutex);
ret = steam_send_report(steam, cmd, sizeof(cmd));
if (ret < 0)
return ret;
@@ -537,7 +537,6 @@ static int steam_get_attributes(struct steam_device *steam)
int i;
struct steam_controller_attribute *attr;
- guard(mutex)(&steam->report_mutex);
ret = steam_send_report(steam, cmd, sizeof(cmd));
if (ret < 0)
return ret;
@@ -599,9 +598,7 @@ static inline int steam_haptic_pulse(struct steam_device *steam, u8 pad,
report[8] = count >> 8;
report[9] = gain;
- mutex_lock(&steam->report_mutex);
ret = steam_send_report(steam, report, sizeof(report));
- mutex_unlock(&steam->report_mutex);
return ret;
}
@@ -621,9 +618,7 @@ static inline int steam_haptic_rumble(struct steam_device *steam,
report[9] = left_gain;
report[10] = right_gain;
- mutex_lock(&steam->report_mutex);
ret = steam_send_report(steam, report, sizeof(report));
- mutex_unlock(&steam->report_mutex);
return ret;
}
@@ -631,8 +626,11 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
rumble_work);
+
+ mutex_lock(&steam->report_mutex);
steam_haptic_rumble(steam, 0, steam->rumble_left,
steam->rumble_right, 2, 0);
+ mutex_unlock(&steam->report_mutex);
}
#ifdef CONFIG_STEAM_FF
@@ -653,7 +651,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
if (steam->gamepad_mode)
enable = false;
- mutex_lock(&steam->report_mutex);
if (enable) {
/* enable esc, enter, cursors */
steam_send_report_byte(steam, ID_SET_DEFAULT_DIGITAL_MAPPINGS);
@@ -678,14 +675,13 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
0);
}
}
- mutex_unlock(&steam->report_mutex);
}
static int steam_input_open(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
unsigned long flags;
- bool set_lizard_mode;
+ bool client_opened;
/*
* Disabling lizard mode automatically is only done on the Steam
@@ -694,9 +690,10 @@ static int steam_input_open(struct input_dev *dev)
*/
if (!(steam->quirks & STEAM_QUIRK_DECK)) {
spin_lock_irqsave(&steam->lock, flags);
- set_lizard_mode = !steam->client_opened && lizard_mode;
+ client_opened = steam->client_opened;
spin_unlock_irqrestore(&steam->lock, flags);
- if (set_lizard_mode)
+ guard(mutex)(&steam->report_mutex);
+ if (!client_opened && lizard_mode)
steam_set_lizard_mode(steam, false);
}
@@ -707,13 +704,14 @@ static void steam_input_close(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
unsigned long flags;
- bool set_lizard_mode;
+ bool client_opened;
if (!(steam->quirks & STEAM_QUIRK_DECK)) {
spin_lock_irqsave(&steam->lock, flags);
- set_lizard_mode = !steam->client_opened && lizard_mode;
+ client_opened = steam->client_opened;
spin_unlock_irqrestore(&steam->lock, flags);
- if (set_lizard_mode)
+ guard(mutex)(&steam->report_mutex);
+ if (!client_opened && lizard_mode)
steam_set_lizard_mode(steam, true);
}
}
@@ -721,14 +719,11 @@ static void steam_input_close(struct input_dev *dev)
static int steam_sensor_open(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
- unsigned long flags;
- bool client_opened;
- spin_lock_irqsave(&steam->lock, flags);
- client_opened = steam->client_opened;
- spin_unlock_irqrestore(&steam->lock, flags);
- if (client_opened)
- return 0;
+ scoped_guard(spinlock_irqsave, &steam->lock) {
+ if (steam->client_opened)
+ return 0;
+ }
guard(mutex)(&steam->report_mutex);
steam_write_settings(steam, SETTING_IMU_MODE,
@@ -741,14 +736,11 @@ static int steam_sensor_open(struct input_dev *dev)
static void steam_sensor_close(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
- unsigned long flags;
- bool client_opened;
- spin_lock_irqsave(&steam->lock, flags);
- client_opened = steam->client_opened;
- spin_unlock_irqrestore(&steam->lock, flags);
- if (client_opened)
- return;
+ scoped_guard(spinlock_irqsave, &steam->lock) {
+ if (steam->client_opened)
+ return;
+ }
guard(mutex)(&steam->report_mutex);
steam_write_settings(steam, SETTING_IMU_MODE, 0, 0);
@@ -1090,15 +1082,19 @@ static int steam_register(struct steam_device *steam)
{
int ret;
+ mutex_lock(&steam->registration_mutex);
/*
* This function can be called several times in a row with the
* wireless adaptor, without steam_unregister() between them, because
* another client send a get_connection_status command, for example.
* The battery and serial number are set just once per device.
*/
- if (steam->registered)
+ if (steam->registered) {
+ mutex_unlock(&steam->registration_mutex);
return 0;
+ }
+ mutex_lock(&steam->report_mutex);
/*
* Unlikely, but getting the serial could fail, and it is not so
* important, so make up a serial number and go on.
@@ -1116,11 +1112,13 @@ static int steam_register(struct steam_device *steam)
hid_info(steam->hdev, "Steam Controller '%s' connected",
steam->serial_no);
+ steam_set_lizard_mode(steam, lizard_mode);
+ mutex_unlock(&steam->report_mutex);
+
/* ignore battery errors, we can live without it */
if (steam->quirks & STEAM_QUIRK_WIRELESS)
steam_battery_register(steam);
- steam_set_lizard_mode(steam, lizard_mode);
ret = steam_input_register(steam);
if (ret != 0)
goto steam_register_input_fail;
@@ -1129,6 +1127,7 @@ static int steam_register(struct steam_device *steam)
goto steam_register_sensors_fail;
steam->registered = true;
+ mutex_unlock(&steam->registration_mutex);
mutex_lock(&steam_devices_lock);
if (list_empty(&steam->list))
list_add(&steam->list, &steam_devices);
@@ -1139,6 +1138,7 @@ static int steam_register(struct steam_device *steam)
steam_input_unregister(steam);
steam_register_input_fail:
steam_battery_unregister(steam);
+ mutex_unlock(&steam->registration_mutex);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->mode_switch);
return ret;
@@ -1151,10 +1151,12 @@ static void steam_unregister(struct steam_device *steam)
hid_info(steam->hdev, "Steam Controller '%s' disconnected",
steam->serial_no);
+ mutex_lock(&steam->registration_mutex);
steam->registered = false;
steam_battery_unregister(steam);
steam_sensors_unregister(steam);
steam_input_unregister(steam);
+ mutex_unlock(&steam->registration_mutex);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->mode_switch);
mutex_lock(&steam_devices_lock);
@@ -1195,23 +1197,26 @@ static void steam_mode_switch_cb(struct work_struct *work)
struct steam_device, mode_switch);
unsigned long flags;
bool client_opened;
+ bool gamepad_mode;
+
if (!lizard_mode)
return;
+ spin_lock_irqsave(&steam->lock, flags);
steam->gamepad_mode = !steam->gamepad_mode;
- hid_dbg(steam->hdev, "%s: switching gamepad mode to %i\n", __func__, steam->gamepad_mode);
- if (steam->gamepad_mode)
+ gamepad_mode = steam->gamepad_mode;
+ client_opened = steam->client_opened;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ guard(mutex)(&steam->report_mutex);
+ hid_dbg(steam->hdev, "%s: switching gamepad mode to %i\n", __func__, gamepad_mode);
+ if (gamepad_mode)
steam_set_lizard_mode(steam, false);
- else {
- spin_lock_irqsave(&steam->lock, flags);
- client_opened = steam->client_opened;
- spin_unlock_irqrestore(&steam->lock, flags);
- if (!client_opened)
- steam_set_lizard_mode(steam, lizard_mode);
- }
+ else if (!client_opened)
+ steam_set_lizard_mode(steam, lizard_mode);
steam_haptic_pulse(steam, STEAM_PAD_RIGHT, 0x190, 0, 1, 0);
- if (steam->gamepad_mode) {
+ if (gamepad_mode) {
steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x14D, 0x14D, 0x2D, 0);
} else {
steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x1F4, 0x1F4, 0x1E, 0);
@@ -1364,6 +1369,7 @@ static int steam_probe(struct hid_device *hdev,
hid_set_drvdata(hdev, steam);
spin_lock_init(&steam->lock);
mutex_init(&steam->report_mutex);
+ mutex_init(&steam->registration_mutex);
steam->quirks = id->driver_data;
INIT_WORK(&steam->work_connect, steam_work_connect_cb);
INIT_DELAYED_WORK(&steam->mode_switch, steam_mode_switch_cb);
@@ -1463,13 +1469,10 @@ static void steam_remove(struct hid_device *hdev)
static void steam_do_connect_event(struct steam_device *steam, bool connected)
{
- unsigned long flags;
bool changed;
- spin_lock_irqsave(&steam->lock, flags);
changed = steam->connected != connected;
steam->connected = connected;
- spin_unlock_irqrestore(&steam->lock, flags);
if (changed && schedule_work(&steam->work_connect) == 0)
dbg_hid("%s: connected=%d event already queued\n",
@@ -1898,8 +1901,6 @@ static void steam_do_deck_sensors_event(struct steam_device *steam,
static void steam_do_battery_event(struct steam_device *steam,
struct power_supply *battery, u8 *data)
{
- unsigned long flags;
-
s16 volts = steam_le16(data + 12);
u8 batt = data[14];
@@ -1907,10 +1908,8 @@ static void steam_do_battery_event(struct steam_device *steam,
rcu_read_lock();
battery = rcu_dereference(steam->battery);
if (likely(battery)) {
- spin_lock_irqsave(&steam->lock, flags);
steam->voltage = volts;
steam->battery_charge = batt;
- spin_unlock_irqrestore(&steam->lock, flags);
power_supply_changed(battery);
}
rcu_read_unlock();
@@ -1928,6 +1927,7 @@ static int steam_raw_event(struct hid_device *hdev,
if (!steam)
return 0;
+ guard(spinlock_irqsave)(&steam->lock);
if (steam->client_opened)
hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
data, size, 0);
@@ -2015,6 +2015,8 @@ static int steam_param_set_lizard_mode(const char *val,
{
struct steam_device *steam;
int ret;
+ bool client_opened;
+ unsigned long flags;
ret = param_set_bool(val, kp);
if (ret)
@@ -2022,8 +2024,13 @@ static int steam_param_set_lizard_mode(const char *val,
mutex_lock(&steam_devices_lock);
list_for_each_entry(steam, &steam_devices, list) {
- if (!steam->client_opened)
+ spin_lock_irqsave(&steam->lock, flags);
+ client_opened = steam->client_opened;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (!client_opened) {
+ guard(mutex)(&steam->report_mutex);
steam_set_lizard_mode(steam, lizard_mode);
+ }
}
mutex_unlock(&steam_devices_lock);
return 0;
--
2.56.0.385.gd3acb90ef8-goog
prev parent reply other threads:[~2026-10-13 9:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-13 9:11 [STABLE v6.18 1/4] HID: steam: Fully unregister controller when hidraw is opened Lee Jones
2026-10-13 9:11 ` [STABLE v6.18 2/4] HID: steam: Rearrange teardown sequence Lee Jones
2026-10-13 9:11 ` [STABLE v6.18 3/4] HID: steam: Refactor registration Lee Jones
2026-10-13 9:11 ` Lee Jones [this message]
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=20261013091138.526840-4-lee@kernel.org \
--to=lee@kernel.org \
--cc=bentiss@kernel.org \
--cc=jikos@kernel.org \
--cc=jkosina@suse.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=vi@endrift.com \
/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