* [PATCH v3 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
Sensor support was added for the Steam Deck previously, but Steam
Controller sensor events were never added. This adds that missing support,
bringing Steam Controller support much closer to feature parity with things
like SDL and Steam itself.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 214 ++++++++++++++++++++++++++++++++--------
1 file changed, 175 insertions(+), 39 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index add9f4f27fff..6029c055a6cf 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -70,13 +70,14 @@ static LIST_HEAD(steam_devices);
/* Joystick runs are about 5 mm and 32768 units */
#define STEAM_DECK_JOYSTICK_RESOLUTION 6553
/* Accelerometer has 16 bit resolution and a range of +/- 2g */
-#define STEAM_DECK_ACCEL_RES_PER_G 16384
-#define STEAM_DECK_ACCEL_RANGE 32768
+#define STEAM_ACCEL_RES_PER_G 16384
+#define STEAM_ACCEL_RANGE 32768
+#define STEAM_ACCEL_FUZZ 128
#define STEAM_DECK_ACCEL_FUZZ 32
/* Gyroscope has 16 bit resolution and a range of +/- 2000 dps */
-#define STEAM_DECK_GYRO_RES_PER_DPS 16
-#define STEAM_DECK_GYRO_RANGE 32768
-#define STEAM_DECK_GYRO_FUZZ 1
+#define STEAM_GYRO_RES_PER_DPS 16
+#define STEAM_GYRO_RANGE 32768
+#define STEAM_GYRO_FUZZ 0
#define STEAM_PAD_FUZZ 256
@@ -255,6 +256,31 @@ enum
ID_CONTROLLER_DECK_STATE = 9
};
+/* Read-only attributes */
+enum {
+ ATTRIB_UNIQUE_ID, // deprecated
+ ATTRIB_PRODUCT_ID,
+ ATTRIB_PRODUCT_REVISON, // deprecated
+ ATTRIB_CAPABILITIES = ATTRIB_PRODUCT_REVISON, // intentional aliasing
+ ATTRIB_FIRMWARE_VERSION, // deprecated
+ ATTRIB_FIRMWARE_BUILD_TIME,
+ ATTRIB_RADIO_FIRMWARE_BUILD_TIME,
+ ATTRIB_RADIO_DEVICE_ID0,
+ ATTRIB_RADIO_DEVICE_ID1,
+ ATTRIB_DONGLE_FIRMWARE_BUILD_TIME,
+ ATTRIB_HW_ID, // AKA BOARD_REVISION,
+ ATTRIB_BOOTLOADER_BUILD_TIME,
+ ATTRIB_CONNECTION_INTERVAL_IN_US,
+ ATTRIB_SECONDARY_FIRMWARE_BUILD_TIME,
+ ATTRIB_SECONDARY_BOOTLOADER_BUILD_TIME,
+ ATTRIB_SECONDARY_HW_ID, // AKA BOARD_REVISION,
+ ATTRIB_STREAMING,
+ ATTRIB_TRACKPAD_ID,
+ ATTRIB_SECONDARY_TRACKPAD_ID,
+
+ ATTRIB_COUNT
+};
+
/* String attribute identifiers */
enum {
ATTRIB_STR_BOARD_SERIAL,
@@ -284,6 +310,11 @@ enum {
TRACKPAD_GESTURE_KEYBOARD,
};
+struct steam_controller_attribute {
+ unsigned char tag;
+ __le32 value;
+} __packed;
+
/* Pad identifiers for the deck */
#define STEAM_PAD_LEFT 0
#define STEAM_PAD_RIGHT 1
@@ -315,6 +346,7 @@ struct steam_device {
u16 rumble_left;
u16 rumble_right;
unsigned int sensor_timestamp_us;
+ unsigned int sensor_update_rate_us;
struct work_struct unregister_work;
};
@@ -468,6 +500,38 @@ static int steam_get_serial(struct steam_device *steam)
return ret;
}
+static int steam_get_attributes(struct steam_device *steam)
+{
+ int ret = 0;
+ u8 cmd[] = {ID_GET_ATTRIBUTES_VALUES, 0};
+ u8 reply[64] = {};
+ u8 size;
+ 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;
+ ret = steam_recv_report(steam, reply, sizeof(reply));
+ if (ret < 0)
+ return ret;
+ if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2)
+ return -EIO;
+
+ size = min(reply[1], sizeof(reply) - 2);
+ for (i = 0; i + sizeof(*attr) <= size; i += sizeof(*attr)) {
+ attr = (struct steam_controller_attribute *)&reply[i + 2];
+ if (attr->tag == ATTRIB_CONNECTION_INTERVAL_IN_US) {
+ steam->sensor_update_rate_us = get_unaligned_le32(&attr->value);
+ hid_dbg(steam->hdev, "Sensor update rate: %uus\n",
+ steam->sensor_update_rate_us);
+ }
+ }
+
+ return 0;
+}
+
/*
* This command requests the wireless adaptor to post an event
* with the connection status. Useful if this driver is loaded when
@@ -626,6 +690,42 @@ 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;
+
+ guard(mutex)(&steam->report_mutex);
+ steam_write_settings(steam, SETTING_IMU_MODE,
+ SETTING_IMU_MODE_SEND_RAW_ACCEL | SETTING_IMU_MODE_SEND_RAW_GYRO,
+ 0);
+
+ return 0;
+}
+
+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;
+
+ guard(mutex)(&steam->report_mutex);
+ steam_write_settings(steam, SETTING_IMU_MODE, 0, 0);
+}
+
static enum power_supply_property steam_battery_props[] = {
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_SCOPE,
@@ -839,9 +939,6 @@ static int steam_sensors_register(struct steam_device *steam)
struct input_dev *sensors;
int ret;
- if (!(steam->quirks & STEAM_QUIRK_DECK))
- return 0;
-
rcu_read_lock();
sensors = rcu_dereference(steam->sensors);
rcu_read_unlock();
@@ -856,8 +953,14 @@ static int steam_sensors_register(struct steam_device *steam)
input_set_drvdata(sensors, steam);
sensors->dev.parent = &hdev->dev;
+ if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+ sensors->open = steam_sensor_open;
+ sensors->close = steam_sensor_close;
+ }
- sensors->name = "Steam Deck Motion Sensors";
+ sensors->name = steam->quirks & STEAM_QUIRK_DECK ?
+ "Steam Deck Motion Sensors" :
+ "Steam Controller Motion Sensors";
sensors->phys = hdev->phys;
sensors->uniq = steam->serial_no;
sensors->id.bustype = hdev->bus;
@@ -869,25 +972,34 @@ static int steam_sensors_register(struct steam_device *steam)
__set_bit(EV_MSC, sensors->evbit);
__set_bit(MSC_TIMESTAMP, sensors->mscbit);
- input_set_abs_params(sensors, ABS_X, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_set_abs_params(sensors, ABS_Y, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_set_abs_params(sensors, ABS_Z, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_abs_set_res(sensors, ABS_X, STEAM_DECK_ACCEL_RES_PER_G);
- input_abs_set_res(sensors, ABS_Y, STEAM_DECK_ACCEL_RES_PER_G);
- input_abs_set_res(sensors, ABS_Z, STEAM_DECK_ACCEL_RES_PER_G);
-
- input_set_abs_params(sensors, ABS_RX, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_set_abs_params(sensors, ABS_RY, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_set_abs_params(sensors, ABS_RZ, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_abs_set_res(sensors, ABS_RX, STEAM_DECK_GYRO_RES_PER_DPS);
- input_abs_set_res(sensors, ABS_RY, STEAM_DECK_GYRO_RES_PER_DPS);
- input_abs_set_res(sensors, ABS_RZ, STEAM_DECK_GYRO_RES_PER_DPS);
+ if (steam->quirks & STEAM_QUIRK_DECK) {
+ input_set_abs_params(sensors, ABS_X, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Y, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Z, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ } else {
+ input_set_abs_params(sensors, ABS_X, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Y, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Z, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ }
+ input_abs_set_res(sensors, ABS_X, STEAM_ACCEL_RES_PER_G);
+ input_abs_set_res(sensors, ABS_Y, STEAM_ACCEL_RES_PER_G);
+ input_abs_set_res(sensors, ABS_Z, STEAM_ACCEL_RES_PER_G);
+
+ input_set_abs_params(sensors, ABS_RX, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_RY, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_RZ, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_abs_set_res(sensors, ABS_RX, STEAM_GYRO_RES_PER_DPS);
+ input_abs_set_res(sensors, ABS_RY, STEAM_GYRO_RES_PER_DPS);
+ input_abs_set_res(sensors, ABS_RZ, STEAM_GYRO_RES_PER_DPS);
ret = input_register_device(sensors);
if (ret)
@@ -918,9 +1030,6 @@ static void steam_sensors_unregister(struct steam_device *steam)
{
struct input_dev *sensors;
- if (!(steam->quirks & STEAM_QUIRK_DECK))
- return;
-
rcu_read_lock();
sensors = rcu_dereference(steam->sensors);
rcu_read_unlock();
@@ -968,6 +1077,12 @@ static int steam_register(struct steam_device *steam)
strscpy(steam->serial_no, "XXXXXXXXXX",
sizeof(steam->serial_no));
+ ret = steam_get_attributes(steam);
+ if (ret < 0)
+ hid_err(steam->hdev,
+ "%s:steam_get_attributes failed with error %d\n",
+ __func__, ret);
+
hid_info(steam->hdev, "Steam Controller '%s' connected",
steam->serial_no);
@@ -1246,6 +1361,10 @@ static int steam_probe(struct hid_device *hdev,
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
steam->sensor_timestamp_us = 0;
+ if (steam->quirks & STEAM_QUIRK_DECK)
+ steam->sensor_update_rate_us = 4000;
+ else
+ steam->sensor_update_rate_us = 9000;
INIT_WORK(&steam->unregister_work, steam_work_unregister_cb);
/*
@@ -1491,6 +1610,16 @@ static const struct steam_axis_mapping steam_controller_axis_mappings[] = {
{ /* sentinel */ },
};
+static const struct steam_axis_mapping steam_controller_imu_mappings[] = {
+ { ABS_X, 1, 28 },
+ { ABS_Z, -1, 30 },
+ { ABS_Y, 1, 32 },
+ { ABS_RX, 1, 34 },
+ { ABS_RZ, 1, 36 },
+ { ABS_RY, 1, 38 },
+ { /* sentinel */ },
+};
+
static void steam_do_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
@@ -1533,6 +1662,17 @@ static void steam_do_input_event(struct steam_device *steam,
input_sync(input);
}
+static void steam_do_sensors_event(struct steam_device *steam,
+ struct input_dev *sensors, u8 *data)
+{
+ steam->sensor_timestamp_us += steam->sensor_update_rate_us;
+
+ input_event(sensors, EV_MSC, MSC_TIMESTAMP, steam->sensor_timestamp_us);
+ steam_map_axes(sensors, steam_controller_imu_mappings, data);
+
+ input_sync(sensors);
+}
+
/*
* The size for this message payload is 56.
* The known values are:
@@ -1727,14 +1867,7 @@ static void steam_do_deck_input_event(struct steam_device *steam,
static void steam_do_deck_sensors_event(struct steam_device *steam,
struct input_dev *sensors, u8 *data)
{
- /*
- * The deck input report is received every 4 ms on average,
- * with a jitter of +/- 4 ms even though the USB descriptor claims
- * that it uses 1 kHz.
- * Since the HID report does not include a sensor timestamp,
- * use a fixed increment here.
- */
- steam->sensor_timestamp_us += 4000;
+ steam->sensor_timestamp_us += steam->sensor_update_rate_us;
if (!steam->gamepad_mode && lizard_mode)
return;
@@ -1819,6 +1952,9 @@ static int steam_raw_event(struct hid_device *hdev,
input = rcu_dereference(steam->input);
if (likely(input))
steam_do_input_event(steam, input, data);
+ sensors = rcu_dereference(steam->sensors);
+ if (likely(sensors))
+ steam_do_sensors_event(steam, sensors, data);
rcu_read_unlock();
break;
case ID_CONTROLLER_DECK_STATE:
--
2.54.0
^ permalink raw reply related
* [PATCH v3 03/10] HID: steam: Rename some constants that got renamed upstream
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
SETTING_MOUSE_POINTER_ENABLED was renamed to SETTING_LIZARD_MODE upstream.
SETTING_GYRO_MODE was renamed to SETTING_IMU_MODE in an older commit, but
the associated enum was overlooked.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 1b0367c98eac..add9f4f27fff 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -151,7 +151,7 @@ enum {
SETTING_USB_DEBUG_MODE,
SETTING_LEFT_TRACKPAD_MODE,
SETTING_RIGHT_TRACKPAD_MODE,
- SETTING_MOUSE_POINTER_ENABLED,
+ SETTING_LIZARD_MODE,
/* 10 */
SETTING_DPAD_DEADZONE,
@@ -261,14 +261,14 @@ enum {
ATTRIB_STR_UNIT_SERIAL,
};
-/* Values for GYRO_MODE (bitmask) */
+/* Values for IMU_MODE (bitmask) */
enum {
- SETTING_GYRO_MODE_OFF = 0,
- SETTING_GYRO_MODE_STEERING = BIT(0),
- SETTING_GYRO_MODE_TILT = BIT(1),
- SETTING_GYRO_MODE_SEND_ORIENTATION = BIT(2),
- SETTING_GYRO_MODE_SEND_RAW_ACCEL = BIT(3),
- SETTING_GYRO_MODE_SEND_RAW_GYRO = BIT(4),
+ SETTING_IMU_MODE_OFF = 0,
+ SETTING_IMU_MODE_STEERING = BIT(0),
+ SETTING_IMU_MODE_TILT = BIT(1),
+ SETTING_IMU_MODE_SEND_ORIENTATION = BIT(2),
+ SETTING_IMU_MODE_SEND_RAW_ACCEL = BIT(3),
+ SETTING_IMU_MODE_SEND_RAW_GYRO = BIT(4),
};
/* Trackpad modes */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 08/10] HID: steam: Improve logging and other cleanup
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
Adds more logging as appropriate, reindents an enum to match surrounding
style, as well as cleaning up some places where we can use guard() instead
of doing locking and unlocking manually.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 56 ++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 9dc77cd76edb..298a2c17e112 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -246,14 +246,14 @@ enum {
/* Input report identifiers */
enum
{
- ID_CONTROLLER_STATE = 1,
- ID_CONTROLLER_DEBUG = 2,
- ID_CONTROLLER_WIRELESS = 3,
- ID_CONTROLLER_STATUS = 4,
- ID_CONTROLLER_DEBUG2 = 5,
- ID_CONTROLLER_SECONDARY_STATE = 6,
- ID_CONTROLLER_BLE_STATE = 7,
- ID_CONTROLLER_DECK_STATE = 9
+ ID_CONTROLLER_STATE = 1,
+ ID_CONTROLLER_DEBUG = 2,
+ ID_CONTROLLER_WIRELESS = 3,
+ ID_CONTROLLER_STATUS = 4,
+ ID_CONTROLLER_DEBUG2 = 5,
+ ID_CONTROLLER_SECONDARY_STATE = 6,
+ ID_CONTROLLER_BLE_STATE = 7,
+ ID_CONTROLLER_DECK_STATE = 9,
};
/* Read-only attributes */
@@ -379,9 +379,16 @@ static int steam_recv_report(struct steam_device *steam,
ret = hid_hw_raw_request(steam->hdev, 0x00,
buf, hid_report_len(r) + 1,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
- if (ret > 0)
- memcpy(data, buf + 1, min(size, ret - 1));
+ if (ret > 0) {
+ ret = min(size, ret - 1);
+ memcpy(data, buf + 1, ret);
+ }
kfree(buf);
+
+ if (ret < 0)
+ hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
+ else
+ hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
return ret;
}
@@ -409,6 +416,8 @@ static int steam_send_report(struct steam_device *steam,
/* The report ID is always 0 */
memcpy(buf + 1, cmd, size);
+ hid_dbg(steam->hdev, "Sending report %*ph\n", size, cmd);
+
/*
* Sometimes the wireless controller fails with EPIPE
* when sending a feature report.
@@ -481,22 +490,21 @@ 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];
- mutex_lock(&steam->report_mutex);
+ guard(mutex)(&steam->report_mutex);
ret = steam_send_report(steam, cmd, sizeof(cmd));
if (ret < 0)
- goto out;
+ return ret;
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
- goto out;
+ return ret;
if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
- ret = -EIO;
- goto out;
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ (int)sizeof(reply), reply);
+ return -EIO;
}
reply[3 + STEAM_SERIAL_LEN] = 0;
strscpy(steam->serial_no, reply + 3, reply[1]);
-out:
- mutex_unlock(&steam->report_mutex);
return ret;
}
@@ -516,8 +524,11 @@ static int steam_get_attributes(struct steam_device *steam)
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
return ret;
- if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2)
+ if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2) {
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ (int)sizeof(reply), reply);
return -EIO;
+ }
size = min(reply[1], sizeof(reply) - 2);
for (i = 0; i + sizeof(*attr) <= size; i += sizeof(*attr)) {
@@ -539,11 +550,8 @@ static int steam_get_attributes(struct steam_device *steam)
*/
static inline int steam_request_conn_status(struct steam_device *steam)
{
- int ret;
- mutex_lock(&steam->report_mutex);
- ret = steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
- mutex_unlock(&steam->report_mutex);
- return ret;
+ guard(mutex)(&steam->report_mutex);
+ return steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
}
/*
@@ -1193,6 +1201,7 @@ static void steam_mode_switch_cb(struct work_struct *work)
return;
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)
steam_set_lizard_mode(steam, false);
else {
@@ -1839,6 +1848,7 @@ static void steam_do_deck_input_event(struct steam_device *steam,
steam->did_mode_switch = false;
cancel_delayed_work(&steam->mode_switch);
} else if (!steam->client_opened && start_pressed && !steam->did_mode_switch) {
+ hid_dbg(steam->hdev, "%s: doing mode switch\n", __func__);
steam->did_mode_switch = true;
schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v3 06/10] HID: steam: Fully unregister controller when hidraw is opened
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
To avoid conflicts between anything touching the hidraw and the driver we
had previously detached the evdev nodes when the hidraw is opened. However,
this isn't sufficient to avoid FEATURE reports from conflicting, so we
change to fully unregistering the controller internally, leaving only the
hidraw active until it's closed.
This also unifies the unregister and connect callbacks, as now the logic
between these two callbacks is identical.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 60 +++++++++++++----------------------------
1 file changed, 18 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 553748813901..0771ad4f5f2c 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -348,7 +348,6 @@ struct steam_device {
u16 rumble_right;
unsigned int sensor_timestamp_us;
unsigned int sensor_update_rate_us;
- struct work_struct unregister_work;
};
static int steam_recv_report(struct steam_device *steam,
@@ -818,6 +817,7 @@ static int steam_battery_register(struct steam_device *steam)
&steam->battery_desc, &battery_cfg);
if (IS_ERR(battery)) {
ret = PTR_ERR(battery);
+ devm_kfree(&steam->hdev->dev, steam->battery_desc.name);
hid_err(steam->hdev,
"%s:power_supply_register failed with error %d\n",
__func__, ret);
@@ -1077,6 +1077,7 @@ static void steam_battery_unregister(struct steam_device *steam)
RCU_INIT_POINTER(steam->battery, NULL);
synchronize_rcu();
power_supply_unregister(battery);
+ devm_kfree(&steam->hdev->dev, steam->battery_desc.name);
}
static int steam_register(struct steam_device *steam)
@@ -1142,38 +1143,41 @@ static int steam_register(struct steam_device *steam)
static void steam_unregister(struct steam_device *steam)
{
+ if (!steam->serial_no[0])
+ return;
+
+ hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+ steam->serial_no);
steam_battery_unregister(steam);
steam_sensors_unregister(steam);
steam_input_unregister(steam);
- if (steam->serial_no[0]) {
- hid_info(steam->hdev, "Steam Controller '%s' disconnected",
- steam->serial_no);
- mutex_lock(&steam_devices_lock);
- list_del_init(&steam->list);
- mutex_unlock(&steam_devices_lock);
- steam->serial_no[0] = 0;
- }
+ mutex_lock(&steam_devices_lock);
+ list_del_init(&steam->list);
+ mutex_unlock(&steam_devices_lock);
+ steam->serial_no[0] = 0;
}
static void steam_work_connect_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
work_connect);
+
unsigned long flags;
bool connected;
+ bool opened;
int ret;
spin_lock_irqsave(&steam->lock, flags);
+ opened = steam->client_opened;
connected = steam->connected;
spin_unlock_irqrestore(&steam->lock, flags);
- if (connected) {
+ if (connected && !opened) {
ret = steam_register(steam);
- if (ret) {
+ if (ret)
hid_err(steam->hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
- }
} else {
steam_unregister(steam);
}
@@ -1207,31 +1211,6 @@ static void steam_mode_switch_cb(struct work_struct *work)
}
}
-static void steam_work_unregister_cb(struct work_struct *work)
-{
- struct steam_device *steam = container_of(work, struct steam_device,
- unregister_work);
- unsigned long flags;
- bool connected;
- bool opened;
-
- spin_lock_irqsave(&steam->lock, flags);
- opened = steam->client_opened;
- connected = steam->connected;
- spin_unlock_irqrestore(&steam->lock, flags);
-
- if (connected) {
- if (opened) {
- steam_sensors_unregister(steam);
- steam_input_unregister(steam);
- } else {
- steam_set_lizard_mode(steam, lizard_mode);
- steam_input_register(steam);
- steam_sensors_register(steam);
- }
- }
-}
-
static bool steam_is_valve_interface(struct hid_device *hdev)
{
struct hid_report_enum *rep_enum;
@@ -1277,7 +1256,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
steam->client_opened++;
spin_unlock_irqrestore(&steam->lock, flags);
- schedule_work(&steam->unregister_work);
+ schedule_work(&steam->work_connect);
return 0;
}
@@ -1292,7 +1271,7 @@ static void steam_client_ll_close(struct hid_device *hdev)
steam->client_opened--;
spin_unlock_irqrestore(&steam->lock, flags);
- schedule_work(&steam->unregister_work);
+ schedule_work(&steam->work_connect);
}
static int steam_client_ll_raw_request(struct hid_device *hdev,
@@ -1389,7 +1368,6 @@ static int steam_probe(struct hid_device *hdev,
steam->sensor_update_rate_us = 4000;
else
steam->sensor_update_rate_us = 9000;
- INIT_WORK(&steam->unregister_work, steam_work_unregister_cb);
/*
* With the real steam controller interface, do not connect hidraw.
@@ -1451,7 +1429,6 @@ static int steam_probe(struct hid_device *hdev,
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
- cancel_work_sync(&steam->unregister_work);
return ret;
}
@@ -1470,7 +1447,6 @@ static void steam_remove(struct hid_device *hdev)
cancel_work_sync(&steam->work_connect);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
- cancel_work_sync(&steam->unregister_work);
steam->client_hdev = NULL;
steam->client_opened = 0;
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
--
2.54.0
^ permalink raw reply related
* [PATCH v3 05/10] HID: steam: Coalesce rumble packets
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
The Steam Deck resets the haptic pattern every time it receives a rumble
packet, leading to weird discontinuities or sometimes cutting out entirely.
Instead of overloading the interface, Steam interally rate-limits sending
these packets, so we should too.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 6029c055a6cf..553748813901 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -343,6 +343,7 @@ struct steam_device {
bool did_mode_switch;
bool gamepad_mode;
struct work_struct rumble_work;
+ struct delayed_work coalesce_rumble_work;
u16 rumble_left;
u16 rumble_right;
unsigned int sensor_timestamp_us;
@@ -603,10 +604,24 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
rumble_work);
+
steam_haptic_rumble(steam, 0, steam->rumble_left,
steam->rumble_right, 2, 0);
}
+static void steam_coalesce_rumble_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(to_delayed_work(work),
+ struct steam_device,
+ coalesce_rumble_work);
+
+ steam_haptic_rumble(steam, 0, steam->rumble_left,
+ steam->rumble_right, 2, 0);
+
+ if (steam->rumble_left || steam->rumble_right)
+ schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
+}
+
#ifdef CONFIG_STEAM_FF
static int steam_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
@@ -616,6 +631,14 @@ static int steam_play_effect(struct input_dev *dev, void *data,
steam->rumble_left = effect->u.rumble.strong_magnitude;
steam->rumble_right = effect->u.rumble.weak_magnitude;
+ /*
+ * The interface gets somewhat overloaded when too many rumble
+ * packets are sent in a row, so Steam throttles it to 20 Hz
+ */
+ if (delayed_work_pending(&steam->coalesce_rumble_work))
+ return 0;
+
+ schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
return schedule_work(&steam->rumble_work);
}
#endif
@@ -1360,6 +1383,7 @@ static int steam_probe(struct hid_device *hdev,
INIT_DELAYED_WORK(&steam->mode_switch, steam_mode_switch_cb);
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
+ INIT_DELAYED_WORK(&steam->coalesce_rumble_work, steam_coalesce_rumble_cb);
steam->sensor_timestamp_us = 0;
if (steam->quirks & STEAM_QUIRK_DECK)
steam->sensor_update_rate_us = 4000;
@@ -1426,6 +1450,7 @@ static int steam_probe(struct hid_device *hdev,
cancel_work_sync(&steam->work_connect);
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
cancel_work_sync(&steam->unregister_work);
return ret;
@@ -1444,6 +1469,7 @@ static void steam_remove(struct hid_device *hdev)
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->work_connect);
cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
cancel_work_sync(&steam->unregister_work);
steam->client_hdev = NULL;
steam->client_opened = 0;
--
2.54.0
^ permalink raw reply related
* [PATCH v3 07/10] HID: steam: Rearrange teardown sequences
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
This fixes a narrow window during the teardown sequences in probe and
remove where callbacks could still be scheduled during cleanup that would then
have a dangling pointer to the now-freed steam struct.
Signed-off-by: Vicki Pfau <vi@endrift.com>
HID: steam: Reorder probe
---
drivers/hid/hid-steam.c | 67 ++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 31 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 0771ad4f5f2c..9dc77cd76edb 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -1328,6 +1328,7 @@ static int steam_probe(struct hid_device *hdev,
{
struct steam_device *steam;
int ret;
+ unsigned long flags;
ret = hid_parse(hdev);
if (ret) {
@@ -1353,6 +1354,14 @@ static int steam_probe(struct hid_device *hdev,
if (!steam)
return -ENOMEM;
+ /*
+ * With the real steam controller interface, do not connect hidraw.
+ * Instead, create the client_hid and connect that.
+ */
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
+ if (ret)
+ return ret;
+
steam->hdev = hdev;
hid_set_drvdata(hdev, steam);
spin_lock_init(&steam->lock);
@@ -1369,22 +1378,6 @@ static int steam_probe(struct hid_device *hdev,
else
steam->sensor_update_rate_us = 9000;
- /*
- * With the real steam controller interface, do not connect hidraw.
- * Instead, create the client_hid and connect that.
- */
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
- if (ret)
- goto err_cancel_work;
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev,
- "%s:hid_hw_open\n",
- __func__);
- goto err_hw_stop;
- }
-
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver connected");
/* If using a wireless adaptor ask for connection status */
@@ -1398,14 +1391,22 @@ static int steam_probe(struct hid_device *hdev,
hid_err(hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
- goto err_hw_close;
+ goto err_hw_stop;
}
}
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_open\n",
+ __func__);
+ goto err_steam_unregister;
+ }
+
steam->client_hdev = steam_create_client_hid(hdev);
if (IS_ERR(steam->client_hdev)) {
ret = PTR_ERR(steam->client_hdev);
- goto err_steam_unregister;
+ goto err_hw_close;
}
steam->client_hdev->driver_data = steam;
@@ -1417,18 +1418,20 @@ static int steam_probe(struct hid_device *hdev,
err_destroy:
hid_destroy_device(steam->client_hdev);
-err_steam_unregister:
- if (steam->connected)
- steam_unregister(steam);
err_hw_close:
hid_hw_close(hdev);
-err_hw_stop:
- hid_hw_stop(hdev);
-err_cancel_work:
+err_steam_unregister:
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->client_opened = 0;
+ spin_unlock_irqrestore(&steam->lock, flags);
cancel_work_sync(&steam->work_connect);
+ if (steam->connected)
+ steam_unregister(steam);
cancel_delayed_work_sync(&steam->mode_switch);
+err_hw_stop:
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
+ hid_hw_stop(hdev);
return ret;
}
@@ -1436,25 +1439,27 @@ static int steam_probe(struct hid_device *hdev,
static void steam_remove(struct hid_device *hdev)
{
struct steam_device *steam = hid_get_drvdata(hdev);
+ unsigned long flags;
if (!steam || hdev->group == HID_GROUP_STEAM) {
hid_hw_stop(hdev);
return;
}
+ hid_hw_close(hdev);
hid_destroy_device(steam->client_hdev);
- cancel_delayed_work_sync(&steam->mode_switch);
- cancel_work_sync(&steam->work_connect);
- cancel_work_sync(&steam->rumble_work);
- cancel_delayed_work_sync(&steam->coalesce_rumble_work);
- steam->client_hdev = NULL;
+ spin_lock_irqsave(&steam->lock, flags);
steam->client_opened = 0;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ cancel_work_sync(&steam->work_connect);
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver disconnected");
}
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
steam_unregister(steam);
+ cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->mode_switch);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
+ hid_hw_stop(hdev);
}
static void steam_do_connect_event(struct steam_device *steam, bool connected)
--
2.54.0
^ permalink raw reply related
* [PATCH v3 09/10] HID: steam: Reject short reads
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: Vicki Pfau, Yousef Alhouseen, syzbot+75f3f9bff8c510602d36
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
Steam Controller FEATURE reports encode the size of the message in the
message itself. Previously we were trusting that the size reported matched
the size we actually read, leading to a potential issue with short reads.
Instead, we should actually verify the length of the read.
Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 298a2c17e112..a80d54ed9784 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -357,6 +357,13 @@ static int steam_recv_report(struct steam_device *steam,
u8 *buf;
int ret;
+ /*
+ * All reports start with a two byte header.
+ * We must read at least two bytes to get a sensible output.
+ */
+ if (size < 2)
+ return -EINVAL;
+
r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
if (!r) {
hid_err(steam->hdev, "No HID_FEATURE_REPORT submitted - nothing to read\n");
@@ -380,16 +387,30 @@ static int steam_recv_report(struct steam_device *steam,
buf, hid_report_len(r) + 1,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
if (ret > 0) {
- ret = min(size, ret - 1);
- memcpy(data, buf + 1, ret);
+ /* Remove the report ID from the return buffer */
+ ret--;
+ size = min(size, ret);
+ memcpy(data, buf + 1, size);
}
kfree(buf);
if (ret < 0)
hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
else
- hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
- return ret;
+ hid_dbg(steam->hdev, "Received report %*ph\n", size, data);
+ if (ret < 0)
+ return ret;
+
+ if (ret < 2) {
+ hid_err(steam->hdev, "%s: reply too short\n", __func__);
+ return -EPROTO;
+ }
+ if (ret < data[1] + 2) {
+ hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
+ __func__, data[1] + 2, ret);
+ return -EPROTO;
+ }
+ return size;
}
static int steam_send_report(struct steam_device *steam,
--
2.54.0
^ permalink raw reply related
* [PATCH v3 10/10] HID: steam: Retry send/recv reports if stale
From: Vicki Pfau @ 2026-07-10 2:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260710020048.2337655-1-vi@endrift.com>
Sometimes recv report will reply with a stale result from a previous send
report. Instead of failing out, we should retry them, as they generally
reply correctly after three tries, give or take.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 54 +++++++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index a80d54ed9784..3f5d4a88b864 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -501,6 +501,43 @@ static int steam_write_settings(struct steam_device *steam,
return steam_recv_report(steam, cmd, 2 + cmd[1]);
}
+static int steam_exchange_report(struct steam_device *steam, u8 *cmd, int csize,
+ u8 *reply, int rsize)
+{
+ unsigned int retries = 5;
+ int ret;
+
+ guard(mutex)(&steam->report_mutex);
+ do {
+ ret = steam_send_report(steam, cmd, csize);
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, rsize);
+ /*
+ * Sometimes this can fail on the first few tries on the Steam
+ * Controller (2015). It appears to be a firmware bug, and Steam
+ * itself just retries, so we should also retry a few times to
+ * see if we get it.
+ */
+ if (ret == -EPROTO)
+ continue;
+ if (ret < 0) {
+ hid_err(steam->hdev, "%s: error reading reply (%*ph)\n",
+ __func__, csize, cmd);
+ return ret;
+ }
+ if (reply[0] == cmd[0] && reply[1] >= 1)
+ break;
+ if (retries > 0)
+ continue;
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ rsize, reply);
+ return -EPROTO;
+ } while (retries--);
+
+ return ret;
+}
+
static int steam_get_serial(struct steam_device *steam)
{
/*
@@ -511,15 +548,10 @@ 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));
+ ret = steam_exchange_report(steam, cmd, sizeof(cmd), reply, sizeof(reply));
if (ret < 0)
return ret;
- ret = steam_recv_report(steam, reply, sizeof(reply));
- if (ret < 0)
- return ret;
- if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
- reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
+ if (reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
(int)sizeof(reply), reply);
return -EIO;
@@ -538,14 +570,10 @@ 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;
- ret = steam_recv_report(steam, reply, sizeof(reply));
+ ret = steam_exchange_report(steam, cmd, sizeof(cmd), reply, sizeof(reply));
if (ret < 0)
return ret;
- if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2) {
+ if (reply[1] < 2) {
hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
(int)sizeof(reply), reply);
return -EIO;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 02/10] HID: steam: Refactor and clean up report parsing
From: sashiko-bot @ 2026-07-10 2:14 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-3-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] Redundant pointer cast discards 'const' qualifier.
Pre-existing issues:
- [High] The teardown sequence in `steam_remove()` cancels workqueues before stopping hardware and input devices, creating a race condition that allows works to be re-queued, leading to a use-after-free.
- [High] Probe error paths skip unregistering the device, leaking resources and corrupting the global `steam_devices` list.
--
--- Patch [2]: [PATCH v3 02/10] HID: steam: Refactor and clean up report parsing ---
commit 09cdda46064db010796f95410b8370beccf669e3
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Refactor and clean up report parsing
This commit refactors the parsing of buttons and axes into a unified
array-based mapping approach rather than parsing each out individually.
It introduces mapping arrays and helper functions to simplify the event
handling code.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index a854d6360a0e3..1b0367c98eac1 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -1355,13 +1356,45 @@ static void steam_do_connect_event(struct steam_device *steam, bool connected)
> * Clamp the values to 32767..-32767 so that the range is
> * symmetrical and can be negated safely.
> */
> -static inline s16 steam_le16(u8 *data)
> +static inline s16 steam_le16(const u8 *data)
> {
> - s16 x = (s16) le16_to_cpup((__le16 *)data);
> + s16 x = (s16) get_unaligned_le16((__le16 *)data);
[Severity: Low]
Since the data parameter is now const, and get_unaligned_le16() natively
accepts a const void pointer, does the explicit cast to (__le16 *)
unnecessarily discard the const qualifier and risk a -Wcast-qual warning?
[ ... ]
[Severity: High]
This is a pre-existing issue, but looking at the teardown sequence in
steam_remove(), background tasks are canceled before unregistering the
device and stopping the hardware:
drivers/hid/hid-steam.c:steam_remove() {
...
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->mode_switch);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
hid_hw_stop(hdev);
}
Is it possible for a use-after-free to occur if an incoming hardware event
triggers steam_do_connect_event() and dynamically re-queues the work items
during this race window?
[Severity: High]
This is also a pre-existing issue, but looking at the error paths in
steam_probe(), could we be leaking resources and corrupting the global
steam_devices list?
If steam_register() adds the device to the list but subsequently fails
(for example, during steam_input_register()), it returns an error while
leaving the struct attached:
drivers/hid/hid-steam.c:steam_probe() {
...
ret = steam_register(steam);
if (ret) {
hid_err(hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
goto err_hw_stop;
}
...
}
The err_hw_stop label completely bypasses err_steam_unregister, leaving a
freed pointer in the steam_devices list which could cause a kernel crash
when iterating the list later.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=2
^ permalink raw reply
* Re: [PATCH v3 06/10] HID: steam: Fully unregister controller when hidraw is opened
From: sashiko-bot @ 2026-07-10 2:17 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-7-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] If `steam_register()` fails during `steam_probe()`, the device is left in the global `steam_devices` list, leading to a Use-After-Free.
- [High] In `steam_remove()`, workqueues are cancelled before stopping the HID hardware and unregistering input devices, allowing works to be re-queued and executed after the device is freed.
--
--- Patch [6]: [PATCH v3 06/10] HID: steam: Fully unregister controller when hidraw is opened ---
commit b7eebc07a61af5a54e2737e2f58726a8f773d095
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Fully unregister controller when hidraw is opened
To avoid conflicts between anything touching the hidraw and the driver we
had previously detached the evdev nodes when the hidraw is opened. However,
this isn't sufficient to avoid FEATURE reports from conflicting, so we
change to fully unregistering the controller internally, leaving only the
hidraw active until it's closed.
This also unifies the unregister and connect callbacks, as now the logic
between these two callbacks is identical.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 5537488139011..0771ad4f5f2c2 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -1451,7 +1429,6 @@ static int steam_probe(struct hid_device *hdev,
> cancel_delayed_work_sync(&steam->mode_switch);
> cancel_work_sync(&steam->rumble_work);
> cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> - cancel_work_sync(&steam->unregister_work);
>
> return ret;
> }
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a failed
probe to leak the device pointer in the global steam_devices list?
If steam_register() fails during a wired controller's steam_probe(), the
failure causes steam_probe() to jump to an error label that bypasses
err_steam_unregister, which would have cleanly removed it via
steam_unregister().
The devres subsystem then frees the steam structure. Any subsequent iteration
of steam_devices—such as modifying the lizard_mode module parameter via sysfs
or plugging in another controller—will dereference this freed memory.
> @@ -1470,7 +1447,6 @@ static void steam_remove(struct hid_device *hdev)
> cancel_work_sync(&steam->work_connect);
> cancel_work_sync(&steam->rumble_work);
> cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> - cancel_work_sync(&steam->unregister_work);
> steam->client_hdev = NULL;
> steam->client_opened = 0;
[Severity: High]
This is a pre-existing issue, but does cancelling the workqueues here create
a race condition?
In steam_remove(), workqueues (like rumble_work and work_connect) are
cancelled early in the teardown sequence. However, hid_hw_stop() is called
after these cancellations, meaning the HID transport is still active and
incoming reports can trigger steam_do_connect_event(), which re-queues
work_connect.
Furthermore, steam_unregister() is called at the very end of steam_remove().
Because the input device is still active, userspace can send force-feedback
events that call steam_play_effect(), re-queuing rumble_work and
coalesce_rumble_work.
Since these works are re-queued after the cancel_work_sync() calls, could
they execute after the steam structure is freed by devres, causing a
Use-After-Free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=6
^ permalink raw reply
* Re: [PATCH v3 08/10] HID: steam: Improve logging and other cleanup
From: sashiko-bot @ 2026-07-10 2:17 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-9-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The patch introduces a kernel stack information leak by logging uninitialized memory in the error path of `steam_get_serial`.
Pre-existing issues:
- [High] `steam_get_serial()` does not verify the length of the received report before reading from the uninitialized `reply` buffer, leaking kernel stack memory into the device serial number.
- [High] Infinite workqueue reschedule loop if the controller disconnects while force feedback is active.
--
--- Patch [8]: [PATCH v3 08/10] HID: steam: Improve logging and other cleanup ---
commit 077388dea9c00bbb5cfcdddffe77daaebbb2a614
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: steam: Improve logging and other cleanup
This commit adds more logging as appropriate, reindents an enum to match
surrounding style, and replaces manual lock/unlock sequences with scope-based
guard() constructs.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -481,22 +490,21 @@ 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];
>
> - mutex_lock(&steam->report_mutex);
> + guard(mutex)(&steam->report_mutex);
> ret = steam_send_report(steam, cmd, sizeof(cmd));
> if (ret < 0)
> - goto out;
> + return ret;
> ret = steam_recv_report(steam, reply, sizeof(reply));
> if (ret < 0)
> - goto out;
> + return ret;
> if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
[Severity: High]
This is a pre-existing issue, but can this validation block read out-of-bounds
uninitialized memory?
If a device returns a short report (e.g., 1 byte), steam_recv_report()
returns 1, leaving reply[1] and reply[2] uninitialized.
If these uninitialized bytes happen to match the validation criteria, could
strscpy() copy up to 21 bytes of uninitialized stack memory into
steam->serial_no, which is then exposed to userspace via the input subsystem?
> - ret = -EIO;
> - goto out;
> + hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
> + (int)sizeof(reply), reply);
[Severity: High]
Could this leak uninitialized stack memory to dmesg?
If steam_recv_report() returns a short report (e.g., fewer bytes than
sizeof(reply)), the reply buffer won't be fully populated.
The newly added hid_err() unconditionally prints the entire sizeof(reply)
array. Does this expose uninitialized kernel stack data to the kernel ring
buffer?
> + return -EIO;
> }
> reply[3 + STEAM_SERIAL_LEN] = 0;
> strscpy(steam->serial_no, reply + 3, reply[1]);
> -out:
> - mutex_unlock(&steam->report_mutex);
> return ret;
> }
[ ... ]
> @@ -1193,6 +1201,7 @@ static void steam_unregister(struct steam_device *steam)
> {
> if (!steam->serial_no[0])
> return;
>
> hid_info(steam->hdev, "Steam Controller '%s' disconnected",
> steam->serial_no);
> steam_battery_unregister(steam);
> steam_sensors_unregister(steam);
> steam_input_unregister(steam);
[Severity: High]
This is a pre-existing issue, but is there a risk of an infinite loop if the
controller disconnects while force feedback is active?
In steam_coalesce_rumble_cb(), the work continuously reschedules itself based
on the non-zero state of rumble_left or rumble_right:
steam_coalesce_rumble_cb()
if (steam->rumble_left || steam->rumble_right)
schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
When steam_unregister() is called on device disconnect, it unregisters the
input devices but doesn't appear to cancel the active force feedback works or
clear their persistent magnitude state.
Could this cause the workqueue to infinitely wake up and send USB reports to
the dongle until it's physically unplugged?
> mutex_lock(&steam_devices_lock);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=8
^ permalink raw reply
* Re: [PATCH v3 07/10] HID: steam: Rearrange teardown sequences
From: sashiko-bot @ 2026-07-10 2:18 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-8-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] Moving hid_hw_start before initialization creates a window for Use-Before-Initialization
- [Critical] Probe error path skips cancellation of work_connect and mode_switch
Pre-existing issues:
- [Critical] Hardware interrupts can re-queue work_connect after it is cancelled during teardown
- [Critical] Failure inside steam_register leaves dangling pointers in a global list
- [Critical] Incoming reports can access a freed client_hdev due to lockless state checking
--
--- Patch [7]: [PATCH v3 07/10] HID: steam: Rearrange teardown sequences ---
commit 9242f209b3856143cba9d679406bd9c9784d93c2
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Rearrange teardown sequences
This patch fixes a narrow window during the teardown sequences in probe
and remove where callbacks could still be scheduled during cleanup. It
reorders the teardown steps to prevent dangling pointers to the freed
steam struct.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 0771ad4f5f2c2..9dc77cd76edb0 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -1353,6 +1354,14 @@ static int steam_probe(struct hid_device *hdev,
> if (!steam)
> return -ENOMEM;
>
> + /*
> + * With the real steam controller interface, do not connect hidraw.
> + * Instead, create the client_hid and connect that.
> + */
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
> +
> steam->hdev = hdev;
> hid_set_drvdata(hdev, steam);
> spin_lock_init(&steam->lock);
[Severity: Critical]
Does calling hid_hw_start() here enable hardware interrupts before the
steam struct is fully initialized?
If an unprompted wireless connect event arrives immediately after
hid_set_drvdata(), could steam_raw_event() -> steam_do_connect_event()
attempt to acquire the uninitialized steam->lock and pass a zero-initialized
steam->work_connect to the workqueue?
[ ... ]
> @@ -1398,14 +1391,22 @@ static int steam_probe(struct hid_device *hdev,
> hid_err(hdev,
> "%s:steam_register failed with error %d\n",
> __func__, ret);
> - goto err_hw_close;
> + goto err_hw_stop;
> }
> }
[Severity: Critical]
If steam_register() fails and jumps directly to err_hw_stop, does this bypass
the cancellation of work_connect and mode_switch at the err_steam_unregister
label?
Since hid_hw_start() is now called earlier, could an incoming interrupt have
already scheduled these works, resulting in a use-after-free when the steam
struct is freed?
[Severity: Critical]
This is a pre-existing issue, but if steam_register() fails here:
drivers/hid/hid-steam.c:steam_register() {
...
mutex_lock(&steam_devices_lock);
if (list_empty(&steam->list))
list_add(&steam->list, &steam_devices);
mutex_unlock(&steam_devices_lock);
...
if (!client_opened) {
ret = steam_input_register(steam);
if (ret != 0)
goto steam_register_input_fail;
...
}
Since the error path jumps to err_hw_stop, bypassing steam_unregister(),
does this leave a dangling pointer to the freed steam struct in the global
steam_devices list?
[ ... ]
> @@ -1436,25 +1439,27 @@ static int steam_probe(struct hid_device *hdev,
> static void steam_remove(struct hid_device *hdev)
> {
> struct steam_device *steam = hid_get_drvdata(hdev);
> + unsigned long flags;
>
> if (!steam || hdev->group == HID_GROUP_STEAM) {
> hid_hw_stop(hdev);
> return;
> }
>
> + hid_hw_close(hdev);
> hid_destroy_device(steam->client_hdev);
> - cancel_delayed_work_sync(&steam->mode_switch);
> - cancel_work_sync(&steam->work_connect);
> - cancel_work_sync(&steam->rumble_work);
> - cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> - steam->client_hdev = NULL;
> + spin_lock_irqsave(&steam->lock, flags);
> steam->client_opened = 0;
> + spin_unlock_irqrestore(&steam->lock, flags);
[Severity: Critical]
This isn't a bug introduced by this patch, but calling
hid_destroy_device(steam->client_hdev) before clearing steam->client_opened
seems risky.
Since steam_raw_event() checks client_opened without holding locks or memory
barriers:
drivers/hid/hid-steam.c:steam_raw_event() {
...
if (steam->client_opened)
hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
data, size, 0);
...
}
Could a concurrent interrupt read a stale non-zero value for client_opened
and access the already-freed client_hdev?
> + cancel_work_sync(&steam->work_connect);
> if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> hid_info(hdev, "Steam wireless receiver disconnected");
> }
> - hid_hw_close(hdev);
> - hid_hw_stop(hdev);
> steam_unregister(steam);
> + cancel_work_sync(&steam->rumble_work);
> + cancel_delayed_work_sync(&steam->mode_switch);
> + cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> + hid_hw_stop(hdev);
> }
[Severity: Critical]
This is a pre-existing issue, but does calling
cancel_work_sync(&steam->work_connect) while hardware interrupts are still
active leave a race window?
If another user has the device open, hid_hw_close() doesn't fully stop the
transport interrupts until hid_hw_stop() is called.
Could an incoming ID_CONTROLLER_WIRELESS report trigger
steam_do_connect_event() and unconditionally re-queue work_connect after it
was cancelled, leading to a use-after-free when the steam struct is freed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=7
^ permalink raw reply
* Re: [PATCH v3 05/10] HID: steam: Coalesce rumble packets
From: sashiko-bot @ 2026-07-10 2:19 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260710020048.2337655-6-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Unconditional self-rescheduling causes severe haptic stutter by spamming redundant packets at 20Hz.
Pre-existing issues:
- [High] Use-after-free during device removal when a force-feedback effect is active.
--
--- Patch [5]: [PATCH v3 05/10] HID: steam: Coalesce rumble packets ---
commit 8356fb642b10f58402cd235e2f8093f10a60cd43
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Coalesce rumble packets
The Steam Deck resets the haptic pattern every time it receives a rumble
packet. This patch rate-limits sending these packets to 20Hz to prevent
discontinuities and cutting out.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 6029c055a6cf3..5537488139011 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -603,10 +604,24 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
> {
> struct steam_device *steam = container_of(work, struct steam_device,
> rumble_work);
> +
> steam_haptic_rumble(steam, 0, steam->rumble_left,
> steam->rumble_right, 2, 0);
> }
>
> +static void steam_coalesce_rumble_cb(struct work_struct *work)
> +{
> + struct steam_device *steam = container_of(to_delayed_work(work),
> + struct steam_device,
> + coalesce_rumble_work);
> +
> + steam_haptic_rumble(steam, 0, steam->rumble_left,
> + steam->rumble_right, 2, 0);
> +
> + if (steam->rumble_left || steam->rumble_right)
> + schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
> +}
[Severity: Medium]
Will this unconditionally self-reschedule and send identical packets at 20Hz
as long as a continuous force-feedback effect is active?
If a constant rumble effect is playing, this worker will continuously send
hardware reports and reschedule itself at 20Hz even if the magnitude has not
changed.
Since the commit message notes that the Steam Deck resets its haptic pattern
upon receiving a rumble packet, won't spamming these redundant packets force
continuous resets and cause the exact haptic stutter this patch is trying to
fix?
Could we add a check to only send packets and reschedule if the requested
rumble magnitudes have actually changed?
[ ... ]
> @@ -1444,6 +1469,7 @@ static void steam_remove(struct hid_device *hdev)
> cancel_delayed_work_sync(&steam->mode_switch);
> cancel_work_sync(&steam->work_connect);
> cancel_work_sync(&steam->rumble_work);
> + cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> cancel_work_sync(&steam->unregister_work);
> steam->client_hdev = NULL;
> steam->client_opened = 0;
[Severity: High]
This is a pre-existing issue, but does canceling these work items before
unregistering the input device cause a use-after-free on device removal?
If the device is physically unplugged while a force-feedback effect is
playing, the removal path cancels the rumble works and then proceeds to call
steam_unregister().
When the input device is unregistered, the input core flushes and stops all
active effects. This operation routes through to steam_play_effect() with a
zero magnitude.
Since steam_play_effect() unconditionally re-arms the work items by calling
schedule_work() and schedule_delayed_work(), the work items we just cancelled
will be queued again. When device removal completes, the steam_device
structure memory is freed by devres.
The re-armed works will execute later and dereference the freed memory. Should
the work items be cancelled after steam_unregister() instead to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=5
^ permalink raw reply
* Re: [PATCH v3 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)
From: sashiko-bot @ 2026-07-10 2:19 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-5-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Medium] The newly added `sensors` IMU configuration is silently destroyed when userspace closes the concurrent `input` evdev node or briefly toggles the hidraw client.
- [Medium] The IMU gyroscope Y-axis mapping for the Steam Controller has an inverted sign, breaking the right-handed coordinate system.
Pre-existing issues:
- [High] Use-After-Free when `steam_register()` fails during `steam_probe()`.
- [High] Concurrent execution of connection and unregistration work queues causes double-registration and permanent memory leaks of input devices.
- [High] Information Leak: `steam_send_report()` sends uninitialized kernel heap memory to the USB device.
- [High] HID I/O requests are issued after the hardware transport is torn down, violating hardware lifecycle rules.
--
--- Patch [4]: [PATCH v3 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015) ---
commit 00dc87454b13954b455cab385838fde427082fff
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Add support for sensor events on the Steam Controller (2015)
Sensor support was added for the Steam Deck previously, but Steam
Controller sensor events were never added. This adds that missing support,
bringing Steam Controller support much closer to feature parity with things
like SDL and Steam itself.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index add9f4f27fff9..6029c055a6cf3 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -468,6 +500,38 @@ static int steam_get_serial(struct steam_device *steam)
> return ret;
> }
>
> +static int steam_get_attributes(struct steam_device *steam)
> +{
> + int ret = 0;
> + u8 cmd[] = {ID_GET_ATTRIBUTES_VALUES, 0};
> + u8 reply[64] = {};
> + u8 size;
> + int i;
> + struct steam_controller_attribute *attr;
> +
> + guard(mutex)(&steam->report_mutex);
> + ret = steam_send_report(steam, cmd, sizeof(cmd));
[Severity: High]
This is a pre-existing issue, but does steam_send_report() send uninitialized
kernel heap memory to the USB device?
Looking at steam_send_report(), it allocates a buffer using
hid_alloc_report_buf(), which internally calls kmalloc() instead of
kzalloc(). It then copies the payload and sends max(size, 64) + 1 bytes via
hid_hw_raw_request().
When steam_get_attributes() calls this with a 2-byte payload, does this
result in up to 63 bytes of uninitialized kernel memory being transmitted to
the USB device?
> + if (ret < 0)
> + return ret;
[ ... ]
> @@ -626,6 +690,42 @@ 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;
> +
> + guard(mutex)(&steam->report_mutex);
> + steam_write_settings(steam, SETTING_IMU_MODE,
> + SETTING_IMU_MODE_SEND_RAW_ACCEL | SETTING_IMU_MODE_SEND_RAW_GYRO,
> + 0);
[Severity: Medium]
Is the newly added sensors IMU configuration silently destroyed when userspace
closes the concurrent input evdev node?
If userspace has both the sensors node and the input node open, and then
closes the input node, steam_input_close() executes and evaluates
!steam->client_opened && lizard_mode. This calls
steam_set_lizard_mode(steam, true), which sends ID_LOAD_DEFAULT_SETTINGS.
Since ID_LOAD_DEFAULT_SETTINGS entirely resets the controller's hardware
configuration and turns off the IMU, does the sensors node remain open while
the physical IMU silently stops reporting data? Could this same state clobbering
occur if the raw HID client is quickly opened and closed?
> +
> + return 0;
> +}
> +
> +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;
> +
> + guard(mutex)(&steam->report_mutex);
> + steam_write_settings(steam, SETTING_IMU_MODE, 0, 0);
[Severity: High]
This is a pre-existing issue, but does calling steam_write_settings() here
violate hardware lifecycle rules during device removal?
In steam_remove(), the driver calls hid_hw_close(hdev) and
hid_hw_stop(hdev), which tears down the USB transport layer. Following this,
steam_unregister() invokes steam_sensors_unregister(), which calls
input_unregister_device().
If a userspace application holds the evdev node open, this triggers the
steam_sensor_close() callback. Since steam_write_settings() issues I/O
requests to a stopped HID device, can this access torn-down transport structures
and cause use-after-free conditions or kernel panics during device unplug?
> +}
[ ... ]
> @@ -839,9 +939,6 @@ static int steam_sensors_register(struct steam_device *steam)
> struct input_dev *sensors;
> int ret;
>
> - if (!(steam->quirks & STEAM_QUIRK_DECK))
> - return 0;
> -
> rcu_read_lock();
> sensors = rcu_dereference(steam->sensors);
> rcu_read_unlock();
> if (sensors) {
> dbg_hid("%s: already connected\n", __func__);
> return 0;
> }
>
> sensors = input_allocate_device();
[Severity: High]
This is a pre-existing issue, but can concurrent execution of the connection and
unregistration work queues cause double-registration and permanent memory leaks?
steam_work_connect_cb() and steam_work_unregister_cb() run on different
workqueues. If they execute concurrently, they can both read steam->sensors
as NULL here because the check only holds rcu_read_lock() and provides no
mutual exclusion.
Could both threads allocate a new device, call input_register_device(), and
publish their pointer via rcu_assign_pointer(), leaving the first device
permanently orphaned and leaked?
> if (!sensors)
[ ... ]
> @@ -968,6 +1077,12 @@ static int steam_register(struct steam_device *steam)
> strscpy(steam->serial_no, "XXXXXXXXXX",
> sizeof(steam->serial_no));
>
> + ret = steam_get_attributes(steam);
> + if (ret < 0)
> + hid_err(steam->hdev,
> + "%s:steam_get_attributes failed with error %d\n",
> + __func__, ret);
> +
> hid_info(steam->hdev, "Steam Controller '%s' connected",
> steam->serial_no);
[Severity: High]
This is a pre-existing issue, but does a failure in steam_register() lead to
a Use-After-Free during steam_probe()?
Early in steam_register(), the steam object is added to the global
steam_devices list. If steam_register() fails, it returns the error code
without removing steam from the list.
Back in steam_probe(), the error is caught, and the code jumps to
goto err_hw_close;, bypassing the err_steam_unregister: label.
steam_probe() exits with an error, and the driver core automatically frees
the steam object allocated with devm_kzalloc().
Does the freed object remain in the global steam_devices list, where any
subsequent read or write to the lizard_mode sysfs parameter would dereference
the freed pointer?
>
[ ... ]
> @@ -1491,6 +1610,16 @@ static const struct steam_axis_mapping steam_controller_axis_mappings[] = {
> { /* sentinel */ },
> };
>
> +static const struct steam_axis_mapping steam_controller_imu_mappings[] = {
> + { ABS_X, 1, 28 },
> + { ABS_Z, -1, 30 },
> + { ABS_Y, 1, 32 },
> + { ABS_RX, 1, 34 },
> + { ABS_RZ, 1, 36 },
[Severity: Medium]
Does the ABS_RZ mapping have an inverted sign here, breaking the right-handed
coordinate system?
The Accel Y-axis is mapped to ABS_Z with a sign of -1, which matches the
Steam Deck mapping, indicating the physical IMU orientation is identical. On the
Steam Deck, the Gyro Y-axis is mapped to ABS_RZ with a sign of -1 to
maintain the correct coordinate system.
Since the Steam Controller mapping uses 1 for Gyro Y (ABS_RZ) instead of
-1, will this report mathematically inverted yaw rotation to applications?
> + { ABS_RY, 1, 38 },
> + { /* sentinel */ },
> +};
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=4
^ permalink raw reply
* Re: [PATCH v3 09/10] HID: steam: Reject short reads
From: sashiko-bot @ 2026-07-10 2:27 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260710020048.2337655-10-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] If `hid_hw_raw_request` returns 0, the function prints uninitialized caller stack memory to the kernel debug log.
Pre-existing issues:
- [High] The driver enables hardware events via `hid_hw_start()` before initializing locks and works, leading to TOCTOU and uninitialized lock usage.
- [Medium] The newly added length validation `ret < data[1] + 2` is insufficient for `steam_get_serial`, leaving an uninitialized stack byte read via `strscpy`.
--
--- Patch [9]: [PATCH v3 09/10] HID: steam: Reject short reads ---
commit 9194c57f44b65844dcf5d6df8e1faa2846607590
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: steam: Reject short reads
This commit adds length validation to steam_recv_report() to ensure that the
reported size of a Steam Controller message matches the bytes actually read.
It also addresses stripping the report ID from the buffer more explicitly.
Link: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 298a2c17e1123..a80d54ed9784b 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -380,16 +387,30 @@ static int steam_recv_report(struct steam_device *steam,
> buf, hid_report_len(r) + 1,
> HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> if (ret > 0) {
> - ret = min(size, ret - 1);
> - memcpy(data, buf + 1, ret);
> + /* Remove the report ID from the return buffer */
> + ret--;
> + size = min(size, ret);
> + memcpy(data, buf + 1, size);
> }
> kfree(buf);
>
> if (ret < 0)
> hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
> else
> - hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
> - return ret;
> + hid_dbg(steam->hdev, "Received report %*ph\n", size, data);
[Severity: Medium]
If hid_hw_raw_request() returns 0, the if (ret > 0) block is bypassed,
leaving size unmodified. Will this cause hid_dbg() to print uninitialized
stack memory from the data array?
> + if (ret < 0)
> + return ret;
> +
> + if (ret < 2) {
> + hid_err(steam->hdev, "%s: reply too short\n", __func__);
> + return -EPROTO;
> + }
> + if (ret < data[1] + 2) {
> + hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
> + __func__, data[1] + 2, ret);
> + return -EPROTO;
> + }
[Severity: Medium]
This is a pre-existing issue, but is this length validation sufficient for
steam_get_serial()?
If a device sends a response where the returned length exactly equals
data[1] + 2, steam_recv_report() accepts the packet.
However, steam_get_serial() calls strscpy(..., reply + 3, reply[1]). This
instructs strscpy() to read up to reply[1] bytes starting from index 3, which
accesses indices 3 through 3 + reply[1] - 1 (which is reply[1] + 2).
Since only indices 0 through data[1] + 1 were validated and initialized, does
this result in an uninitialized stack byte read?
> + return size;
> }
[Severity: High]
This is a pre-existing issue, but does steam_probe() enable hardware events
before initializing locks and workqueues?
In drivers/hid/hid-steam.c:steam_probe():
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
if (ret)
return ret;
steam->hdev = hdev;
hid_set_drvdata(hdev, steam);
spin_lock_init(&steam->lock);
mutex_init(&steam->report_mutex);
steam->quirks = id->driver_data;
INIT_WORK(&steam->work_connect, steam_work_connect_cb);
If a connection event arrives immediately from the hardware after
hid_set_drvdata() but before spin_lock_init() or INIT_WORK(), could
steam_raw_event() attempt to acquire an uninitialized spinlock or schedule
an uninitialized work struct?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=9
^ permalink raw reply
* [PATCH 0/2] HID: hyperv: bound initial device info descriptor
From: Michael Bommarito @ 2026-07-10 2:28 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
stable
A malicious Hyper-V host or backend can crash a guest with a short
SYNTH_HID_INITIAL_DEVICE_INFO message. mousevsc_on_receive_device_info()
trusts the HID descriptor bLength and wDescriptorLength without checking
that the received VMBus packet actually contains both byte ranges, so a
truncated packet with an oversized report-descriptor length makes the
guest read past the received packet while copying the descriptor. This
matters most for a confidential guest, where the host is outside the trust
boundary.
Patch 1 passes the received initial-device-info size into the parser and
rejects descriptor lengths that exceed the packet. Patch 2 adds
same-translation-unit KUnit coverage: a well-formed message that must
still parse and the truncated/oversized message that must now be rejected.
Reproduced with the KUnit/KASAN test: stock reads past the packet on the
short message after the benign control passes; patched rejects it and both
cases pass.
Cc: stable@vger.kernel.org
Michael Bommarito (2):
HID: hyperv: validate initial device info bounds
HID: hyperv: add KUnit coverage for device info bounds
drivers/hid/Kconfig | 10 +++
drivers/hid/hid-hyperv.c | 144 ++++++++++++++++++++++++++++++++++++---
2 files changed, 144 insertions(+), 10 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH 1/2] HID: hyperv: validate initial device info bounds
From: Michael Bommarito @ 2026-07-10 2:28 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
stable
In-Reply-To: <20260710022854.3739558-1-michael.bommarito@gmail.com>
The Hyper-V synthetic HID host supplies SYNTH_HID_INITIAL_DEVICE_INFO
messages that contain a HID descriptor followed by the report descriptor
bytes. mousevsc_on_receive_device_info() trusts bLength and
wDescriptorLength without checking that the received packet contains both
byte ranges.
A malformed host or backend message can therefore make the guest read
past the received VMBus packet while copying the report descriptor. Pass
the received initial-device-info size into the parser and reject
descriptor lengths that exceed the packet.
Impact: A malicious Hyper-V host or backend can crash a guest by sending
a short initial device-info message with an oversized HID report
descriptor length.
Fixes: b95f5bcb811e ("HID: Move the hid-hyperv driver out of staging")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/hid/hid-hyperv.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 7d2b0063df151..fd90196430e29 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -171,18 +171,32 @@ static void mousevsc_free_device(struct mousevsc_dev *device)
}
static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
- struct synthhid_device_info *device_info)
+ struct synthhid_device_info *device_info,
+ u32 device_info_size)
{
int ret = 0;
struct hid_descriptor *desc;
struct mousevsc_prt_msg ack;
+ size_t desc_offset;
+ size_t desc_size;
input_device->dev_info_status = -ENOMEM;
+ if (device_info_size < sizeof(*device_info)) {
+ input_device->dev_info_status = -EINVAL;
+ goto cleanup;
+ }
+
input_device->hid_dev_info = device_info->hid_dev_info;
desc = &device_info->hid_descriptor;
+ desc_offset = offsetof(struct synthhid_device_info, hid_descriptor);
+ desc_size = device_info_size - desc_offset;
if (desc->bLength == 0)
goto cleanup;
+ if (desc->bLength < sizeof(*desc) || desc->bLength > desc_size) {
+ input_device->dev_info_status = -EINVAL;
+ goto cleanup;
+ }
/* The pointer is not NULL when we resume from hibernation */
kfree(input_device->hid_desc);
@@ -197,6 +211,10 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
input_device->dev_info_status = -EINVAL;
goto cleanup;
}
+ if (input_device->report_desc_size > desc_size - desc->bLength) {
+ input_device->dev_info_status = -EINVAL;
+ goto cleanup;
+ }
/* The pointer is not NULL when we resume from hibernation */
kfree(input_device->report_desc);
@@ -273,14 +291,17 @@ static void mousevsc_on_receive(struct hv_device *device,
break;
case SYNTH_HID_INITIAL_DEVICE_INFO:
- WARN_ON(pipe_msg->size < sizeof(struct hv_input_dev_info));
+ if (WARN_ON_ONCE(pipe_msg->size <
+ sizeof(struct synthhid_device_info)))
+ break;
/*
* Parse out the device info into device attr,
* hid desc and report desc
*/
mousevsc_on_receive_device_info(input_dev,
- (struct synthhid_device_info *)pipe_msg->data);
+ (struct synthhid_device_info *)pipe_msg->data,
+ pipe_msg->size);
break;
case SYNTH_HID_INPUT_REPORT:
input_report =
--
2.53.0
^ permalink raw reply related
* [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds
From: Michael Bommarito @ 2026-07-10 2:28 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
stable
In-Reply-To: <20260710022854.3739558-1-michael.bommarito@gmail.com>
Add KUnit coverage for Hyper-V synthetic HID initial device-info parsing.
The tests cover zero bLength, a valid descriptor plus report descriptor,
and a malformed report descriptor length that exceeds the received
message.
The same-translation-unit test uses a KUnit-only ACK bypass so parser
coverage does not require a live VMBus channel.
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
drivers/hid/Kconfig | 10 ++++
drivers/hid/hid-hyperv.c | 117 ++++++++++++++++++++++++++++++++++++---
2 files changed, 120 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index c1d9f7c6a5f23..41ca48d9adc9e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1183,6 +1183,16 @@ config HID_HYPERV_MOUSE
help
Select this option to enable the Hyper-V mouse driver.
+config HID_HYPERV_MOUSE_KUNIT_TEST
+ bool "KUnit tests for Hyper-V mouse driver" if !KUNIT_ALL_TESTS
+ depends on KUNIT && HID_HYPERV_MOUSE
+ default KUNIT_ALL_TESTS
+ help
+ Builds unit tests for the Hyper-V synthetic HID driver.
+ These tests exercise the initial device-info parser with
+ malformed host-provided HID descriptors and are only useful
+ for kernel developers running KUnit.
+
config HID_SMARTJOYPLUS
tristate "SmartJoy PLUS PS2/USB adapter support"
help
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index fd90196430e29..6579bd19da13a 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -13,6 +13,9 @@
#include <linux/hiddev.h>
#include <linux/hyperv.h>
+#if IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST)
+#include <kunit/test.h>
+#endif
struct hv_input_dev_info {
unsigned int size;
@@ -240,13 +243,18 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
ack.ack.header.size = 1;
ack.ack.reserved = 0;
- ret = vmbus_sendpacket(input_device->device->channel,
- &ack,
- sizeof(struct pipe_prt_msg) +
- sizeof(struct synthhid_device_info_ack),
- (unsigned long)&ack,
- VM_PKT_DATA_INBAND,
- VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ if (IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST) &&
+ !input_device->device) {
+ ret = 0;
+ } else {
+ ret = vmbus_sendpacket(input_device->device->channel,
+ &ack,
+ sizeof(struct pipe_prt_msg) +
+ sizeof(struct synthhid_device_info_ack),
+ (unsigned long)&ack,
+ VM_PKT_DATA_INBAND,
+ VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ }
if (!ret)
input_device->dev_info_status = 0;
@@ -635,5 +643,100 @@ static void __exit mousevsc_exit(void)
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Microsoft Hyper-V Synthetic HID Driver");
+#if IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST)
+static struct mousevsc_dev *mousevsc_kunit_alloc_dev(struct kunit *test)
+{
+ struct mousevsc_dev *input_dev;
+
+ input_dev = kunit_kzalloc(test, sizeof(*input_dev), GFP_KERNEL);
+ if (!input_dev)
+ return NULL;
+
+ init_completion(&input_dev->wait_event);
+
+ return input_dev;
+}
+
+static void mousevsc_device_info_zero_blength(struct kunit *test)
+{
+ struct synthhid_device_info *info;
+ struct mousevsc_dev *input_dev;
+
+ input_dev = mousevsc_kunit_alloc_dev(test);
+ KUNIT_ASSERT_NOT_NULL(test, input_dev);
+ info = kunit_kzalloc(test, sizeof(*info), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, info);
+
+ info->hid_descriptor.bLength = 0;
+
+ mousevsc_on_receive_device_info(input_dev, info, sizeof(*info));
+
+ KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -ENOMEM);
+}
+
+static void mousevsc_device_info_valid_descriptor(struct kunit *test)
+{
+ struct synthhid_device_info *info;
+ struct mousevsc_dev *input_dev;
+ u8 *report;
+
+ input_dev = mousevsc_kunit_alloc_dev(test);
+ KUNIT_ASSERT_NOT_NULL(test, input_dev);
+ info = kunit_kzalloc(test, sizeof(*info) + 4, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, info);
+
+ info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
+ info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
+ report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+ memset(report, 0x42, 4);
+
+ mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 4);
+
+ KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, 0);
+ KUNIT_EXPECT_EQ(test, input_dev->report_desc_size, 4);
+ KUNIT_EXPECT_MEMEQ(test, input_dev->report_desc, report, 4);
+
+ kfree(input_dev->hid_desc);
+ kfree(input_dev->report_desc);
+}
+
+static void mousevsc_device_info_report_desc_oob(struct kunit *test)
+{
+ struct synthhid_device_info *info;
+ struct mousevsc_dev *input_dev;
+ u8 *report;
+
+ input_dev = mousevsc_kunit_alloc_dev(test);
+ KUNIT_ASSERT_NOT_NULL(test, input_dev);
+ info = kunit_kzalloc(test, sizeof(*info) + 8, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, info);
+
+ info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
+ info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
+ report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+ memset(report, 0x42, 8);
+
+ mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);
+
+ KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -EINVAL);
+
+ kfree(input_dev->hid_desc);
+}
+
+static struct kunit_case mousevsc_test_cases[] = {
+ KUNIT_CASE(mousevsc_device_info_zero_blength),
+ KUNIT_CASE(mousevsc_device_info_valid_descriptor),
+ KUNIT_CASE(mousevsc_device_info_report_desc_oob),
+ {}
+};
+
+static struct kunit_suite mousevsc_test_suite = {
+ .name = "hid_hyperv_mouse",
+ .test_cases = mousevsc_test_cases,
+};
+
+kunit_test_suite(mousevsc_test_suite);
+#endif
+
module_init(mousevsc_init);
module_exit(mousevsc_exit);
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 10/10] HID: steam: Retry send/recv reports if stale
From: sashiko-bot @ 2026-07-10 2:31 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260710020048.2337655-11-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized kernel stack memory leaked to dmesg on invalid USB replies or short reads.
- [High] Missing cancellation of workqueues in the `steam_probe` error path leads to a Use-After-Free if probe fails.
--
--- Patch [10]: [PATCH v3 10/10] HID: steam: Retry send/recv reports if stale ---
commit 2a2fc2d99039c81756476abdde17aa7642c354be
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Retry send/recv reports if stale
Sometimes recv report will reply with a stale result from a previous send
report. Instead of failing out, we should retry them, as they generally
reply correctly after three tries, give or take.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index a80d54ed9784b..3f5d4a88b8649 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> +static int steam_exchange_report(struct steam_device *steam, u8 *cmd, int csize,
> + u8 *reply, int rsize)
> +{
> + unsigned int retries = 5;
> + int ret;
> +
> + guard(mutex)(&steam->report_mutex);
> + do {
> + ret = steam_send_report(steam, cmd, csize);
> + if (ret < 0)
> + return ret;
> + ret = steam_recv_report(steam, reply, rsize);
[ ... ]
> + if (reply[0] == cmd[0] && reply[1] >= 1)
> + break;
> + if (retries > 0)
> + continue;
> + hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
> + rsize, reply);
[Severity: High]
Does this code leak uninitialized kernel stack memory to dmesg?
If the USB device returns a short reply, steam_recv_report() will only
copy the received bytes into the reply buffer. The remaining bytes in
the buffer remain uninitialized.
Since reply is allocated on the stack in callers like steam_get_serial(),
this hid_err() call will log the full rsize bytes, potentially leaking
sensitive kernel stack memory to dmesg on an invalid or short reply.
> + return -EPROTO;
> + } while (retries--);
> +
> + return ret;
> +}
> +
> static int steam_get_serial(struct steam_device *steam)
> {
> /*
> @@ -511,15 +548,10 @@ 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));
> + ret = steam_exchange_report(steam, cmd, sizeof(cmd), reply, sizeof(reply));
> if (ret < 0)
> return ret;
> - ret = steam_recv_report(steam, reply, sizeof(reply));
> - if (ret < 0)
> - return ret;
> - if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> - reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> + if (reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
> (int)sizeof(reply), reply);
[Severity: High]
Similarly, can this leak uninitialized stack memory to dmesg?
If steam_exchange_report() successfully returns but the reply is invalid or
too short, this error path will log the full sizeof(reply), dumping any
uninitialized stack bytes from the reply buffer into dmesg.
> return -EIO;
> }
[Severity: High]
Does the probe error path cause a Use-After-Free?
Looking at steam_probe() in the same file, if steam_register() fails, the
probe error path jumps to err_hw_stop:
drivers/hid/hid-steam.c:steam_probe() {
...
ret = steam_register(steam);
if (ret) {
hid_err(hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
goto err_hw_stop;
}
...
}
However, this bypasses the cancellation of steam->mode_switch which occurs
under err_steam_unregister:
drivers/hid/hid-steam.c:steam_probe() {
...
err_steam_unregister:
spin_lock_irqsave(&steam->lock, flags);
steam->client_opened = 0;
spin_unlock_irqrestore(&steam->lock, flags);
cancel_work_sync(&steam->work_connect);
if (steam->connected)
steam_unregister(steam);
cancel_delayed_work_sync(&steam->mode_switch);
err_hw_stop:
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
hid_hw_stop(hdev);
...
}
If a malicious USB device sends an input report that triggers
steam_do_deck_input_event() before steam_register() fails, the mode_switch
work is scheduled. When steam_probe() returns the error, the steam struct
will be freed, but the un-canceled mode_switch delayed work could still
execute, causing a UAF on the freed struct.
This teardown issue appears to have been introduced by earlier patches in
this series rearranging the teardown paths without properly covering all
workqueues on the error path.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710020048.2337655-1-vi@endrift.com?part=10
^ permalink raw reply
* Re: Re: [PATCH v2] HID: i2c-hid: Fixes "(null)" device name error logs in i2c_hid_parse
From: Ai Chao @ 2026-07-10 2:39 UTC (permalink / raw)
To: jikos, bentiss, dianders, treapking, kenkinming2002, raoxu,
superm1
Cc: linux-input, linux-kernel
Hello Doug
"Ai" is my surname, and "Chao" is my given name. My name is Ai Chao, which translates to "Super AI".
> > Before: hid (null): reading report descriptor failed
> > After: i2c_hid i2c-TPD0001:00: reading report descriptor failed
> >
> > Signed-off-by: Ai Chao <aichao@kylinos.cn>
> > ---
> > change for v2
> > - Used Fixes tag.
> Could have fooled me. ;-)
syslog:
kernel: [ 3.815131][ 1] [ T319] i2c_designware HYGO0010:00: timeout in disabling adapter
kernel: [ 15.790171][ 1] [ T319] i2c_designware HYGO0010:00: timeout waiting for bus ready
kernel: [ 15.998769][ 1] [ T319] i2c reading report descriptor failed retry:49
kernel: [ 16.038176][ 1] [ T319] i2c_designware HYGO0010:00: timeout waiting for bus ready
kernel: [ 16.246771][ 1] [ T319] i2c reading report descriptor failed retry:50
kernel: [ 16.246775][ 1] [ T319] hid (null): reading report descriptor failed
kernel: [ 16.246778][ 1] [ T319] i2c_hid i2c-TPD0001:00: can't add hid device: -5
kernel: [ 16.246954][ 1] [ T319] i2c_hid: probe of i2c-TPD0001:00 failed with error -5
Thanks,
Ai Chao
^ permalink raw reply
* Re: [PATCH 1/2] HID: hyperv: validate initial device info bounds
From: sashiko-bot @ 2026-07-10 2:40 UTC (permalink / raw)
To: Michael Bommarito; +Cc: linux-hyperv, dmitry.torokhov, linux-input
In-Reply-To: <20260710022854.3739558-2-michael.bommarito@gmail.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Incomplete Vulnerability Fix / Out-of-Bounds Read
- [Medium] Missing Completion / Timeout Hang
- [Low] Dead Code / Redundant Check
Pre-existing issues:
- [High] Race Condition / Use-After-Free
- [High] Out-of-Bounds Access (Read/Write) in Probe Workaround
- [High] Out-of-Bounds Read (Information Leak) in Input Report
--
--- Patch [1]: [PATCH 1/2] HID: hyperv: validate initial device info bounds ---
commit a1876077d0b8322cccf9c09f43d240367749b037
Author: Michael Bommarito <michael.bommarito@gmail.com>
Subject: HID: hyperv: validate initial device info bounds
This commit attempts to fix an out-of-bounds read by passing the received
initial-device-info size into the parser and rejecting descriptor lengths
that exceed the packet.
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -171,18 +171,32 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
> - struct synthhid_device_info *device_info)
> + struct synthhid_device_info *device_info,
> + u32 device_info_size)
> {
> int ret = 0;
> struct hid_descriptor *desc;
> struct mousevsc_prt_msg ack;
> + size_t desc_offset;
> + size_t desc_size;
>
> input_device->dev_info_status = -ENOMEM;
>
> + if (device_info_size < sizeof(*device_info)) {
> + input_device->dev_info_status = -EINVAL;
> + goto cleanup;
> + }
[Severity: Low]
Is this check necessary? In mousevsc_on_receive(), before calling this
function, the code explicitly verifies pipe_msg->size >=
sizeof(struct synthhid_device_info). Since device_info_size is passed
directly from pipe_msg->size, this inner check appears to be unreachable.
> +
> input_device->hid_dev_info = device_info->hid_dev_info;
> desc = &device_info->hid_descriptor;
> + desc_offset = offsetof(struct synthhid_device_info, hid_descriptor);
> + desc_size = device_info_size - desc_offset;
> if (desc->bLength == 0)
> goto cleanup;
> + if (desc->bLength < sizeof(*desc) || desc->bLength > desc_size) {
> + input_device->dev_info_status = -EINVAL;
> + goto cleanup;
> + }
>
> /* The pointer is not NULL when we resume from hibernation */
> kfree(input_device->hid_desc);
> @@ -197,6 +211,10 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
> input_device->dev_info_status = -EINVAL;
> goto cleanup;
> }
> + if (input_device->report_desc_size > desc_size - desc->bLength) {
> + input_device->dev_info_status = -EINVAL;
> + goto cleanup;
> + }
>
> /* The pointer is not NULL when we resume from hibernation */
> kfree(input_device->report_desc);
[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free?
During mousevsc_probe(), the driver waits for the first
SYNTH_HID_INITIAL_DEVICE_INFO and proceeds to hid_add_device(), which parses
input_dev->report_desc.
If the host concurrently sends a second SYNTH_HID_INITIAL_DEVICE_INFO packet,
the channel callback processes it asynchronously and calls
mousevsc_on_receive_device_info(). This executes
kfree(input_device->report_desc) without any locking while the HID core is
actively reading it.
[ ... ]
> @@ -273,14 +291,17 @@ static void mousevsc_on_receive(struct hv_device *device,
> break;
>
> case SYNTH_HID_INITIAL_DEVICE_INFO:
> - WARN_ON(pipe_msg->size < sizeof(struct hv_input_dev_info));
> + if (WARN_ON_ONCE(pipe_msg->size <
> + sizeof(struct synthhid_device_info)))
> + break;
[Severity: Medium]
If this break is taken, are we skipping the completion event?
By breaking out of the switch statement early, we skip the call to
mousevsc_on_receive_device_info(). As a result, the
complete(&input_device->wait_event) inside its cleanup path is never
executed. This might cause the waiting thread in mousevsc_connect_to_vsp()
to stall until the 5-second timeout expires.
>
> /*
> * Parse out the device info into device attr,
> * hid desc and report desc
> */
> mousevsc_on_receive_device_info(input_dev,
> - (struct synthhid_device_info *)pipe_msg->data);
> + (struct synthhid_device_info *)pipe_msg->data,
> + pipe_msg->size);
[Severity: High]
Does this fully prevent out-of-bounds reads if the host sends a malicious
payload?
The pipe_msg->size field comes from the untrusted payload. If a malicious
host sends a short physical VMBus packet but sets pipe_msg->size to an
artificially large value, the bounds checks in
mousevsc_on_receive_device_info() will pass because they are evaluated
against this large device_info_size.
Could we restrict the size to the actual VMBus packet boundary, perhaps using
something like (packet->len8 - packet->offset8) << 3, to ensure we don't
read past the received packet?
> break;
> case SYNTH_HID_INPUT_REPORT:
> input_report =
> (struct synthhid_input_report *)pipe_msg->data;
> if (!input_dev->init_complete)
> break;
>
> len = min(input_report->header.size,
> (u32)sizeof(input_dev->input_buf));
> memcpy(input_dev->input_buf, input_report->buffer, len);
[Severity: High]
This is a pre-existing issue, but does this blind trust of
input_report->header.size enable an out-of-bounds read?
When handling SYNTH_HID_INPUT_REPORT, a malicious host could send a
physically short packet but set input_report->header.size to 4096.
The memcpy length is bounded using:
len = min(input_report->header.size, (u32)sizeof(input_dev->input_buf));
However, it isn't validated against the actual VMBus packet envelope. This
could result in copying beyond the packet boundary, potentially causing an
access violation or exposing memory adjacent to the packet.
[Severity: High]
This is a pre-existing issue, but I noticed another potential out-of-bounds
access during the probe workaround in mousevsc_probe():
drivers/hid/hid-hyperv.c:mousevsc_probe() {
...
if (input_dev->report_desc[14] == 0x25)
input_dev->report_desc[14] = 0x29;
...
}
If a malicious host provides a wDescriptorLength of less than 15 bytes
(for instance, 8 bytes), this accesses and potentially writes to memory past
the dynamically allocated report_desc buffer. Should we ensure the buffer is
at least 15 bytes long before accessing index 14?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022854.3739558-1-michael.bommarito@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds
From: sashiko-bot @ 2026-07-10 2:41 UTC (permalink / raw)
To: Michael Bommarito; +Cc: linux-hyperv, linux-input, dmitry.torokhov
In-Reply-To: <20260710022854.3739558-3-michael.bommarito@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Low] The embedded KUnit test will be silently disabled and cannot be compiled when both KUnit and the Hyper-V mouse driver are configured as modules.
- [Low] Test-specific mock logic is injected directly into the production VMBus I/O path.
Pre-existing issues:
- [High] The driver and the newly added bounds checking rely on unvalidated inner payload sizes (`pipe_msg->size`, `input_report->header.size`) instead of validating against the actual physical VMBus transport packet size, exposing the guest to OOB reads.
- [High] Missing bounds check before accessing `report_desc[14]` allows out-of-bounds heap memory corruption.
--
--- Patch [2]: [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds ---
commit d676a1b2eb911b6f9e0db4e282e431e24c17d123
Author: Michael Bommarito <michael.bommarito@gmail.com>
Subject: HID: hyperv: add KUnit coverage for device info bounds
This commit adds KUnit tests for the Hyper-V synthetic HID initial
device-info parser. The tests exercise malformed host-provided HID
descriptors using a KUnit-only ACK bypass.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1215,6 +1215,16 @@ config HID_HYPERV_MOUSE
[ ... ]
> +config HID_HYPERV_MOUSE_KUNIT_TEST
> + bool "KUnit tests for Hyper-V mouse driver" if !KUNIT_ALL_TESTS
> + depends on KUNIT && HID_HYPERV_MOUSE
[Severity: Low]
When both KUNIT and HID_HYPERV_MOUSE are configured as modules, will this
regression cause the dependency expression to evaluate to "m", forcing the
boolean config to "n" and silently preventing the test from being enabled for
module builds?
The standard kernel idiom for this is "depends on KUNIT=y || KUNIT=HID_HYPERV_MOUSE".
> + default KUNIT_ALL_TESTS
[ ... ]
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
[ ... ]
> @@ -240,13 +243,18 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
[ ... ]
> - ret = vmbus_sendpacket(input_device->device->channel,
> - &ack,
> - sizeof(struct pipe_prt_msg) +
> - sizeof(struct synthhid_device_info_ack),
> - (unsigned long)&ack,
> - VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> + if (IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST) &&
> + !input_device->device) {
[Severity: Low]
Is this regression injecting test-specific mock logic directly into the
production VMBus I/O path?
Could we instead extract the device-info validation into a pure helper
function that the KUnit tests can call directly, avoiding the need to
conditionally bypass network sends in the core logic?
> + ret = 0;
> + } else {
> + ret = vmbus_sendpacket(input_device->device->channel,
[ ... ]
> @@ -635,5 +643,100 @@ static void __exit mousevsc_exit(void)
[ ... ]
> +static void mousevsc_device_info_valid_descriptor(struct kunit *test)
> +{
[ ... ]
> + info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
> + info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
[Severity: High]
This is a pre-existing issue, but does this test highlight an incomplete
bounds check in the driver?
The test sets the report descriptor length to 4 bytes. However, looking at
mousevsc_probe(), the driver contains a workaround that accesses index 14:
mousevsc_probe() {
...
/* workaround SA-167 */
if (input_dev->report_desc[14] == 0x25)
input_dev->report_desc[14] = 0x29;
...
}
If a compromised host provides a valid descriptor with a length between 1 and
14, as tested here, could this result in an out-of-bounds heap memory read
and write since the bounds check in mousevsc_on_receive_device_info() only
ensures the size is greater than zero?
> + report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
[ ... ]
> +static void mousevsc_device_info_report_desc_oob(struct kunit *test)
> +{
[ ... ]
> + info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
> + info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
> + report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
> + memset(report, 0x42, 8);
> +
> + mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);
[Severity: High]
This is a pre-existing issue, but does the bounds validation tested here
rely on an unvalidated inner payload size rather than the physical VMBus
transport packet size?
If a compromised or malicious Hyper-V host sends a VMBus packet with a
massive pipe_msg->size but a small physical payload:
mousevsc_on_receive() {
...
mousevsc_on_receive_device_info(input_dev,
(struct synthhid_device_info *)pipe_msg->data,
pipe_msg->size);
...
}
Could mousevsc_on_receive() pass this spoofable size to
mousevsc_on_receive_device_info(), leading to out-of-bounds reads during
memcpy() or kmemdup() because the bounds checking trusts this inner size?
> +
> + KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -EINVAL);
> +
> + kfree(input_dev->hid_desc);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022854.3739558-1-michael.bommarito@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: Fixes "(null)" device name error logs in i2c_hid_parse
From: Mario Limonciello @ 2026-07-10 4:19 UTC (permalink / raw)
To: Ai Chao, jikos, bentiss, dianders, treapking, kenkinming2002,
raoxu
Cc: linux-input, linux-kernel
In-Reply-To: <20260710023936.3960010-1-aichao@kylinos.cn>
On 7/9/26 21:39, Ai Chao wrote:
> Hello Doug
>
> "Ai" is my surname, and "Chao" is my given name. My name is Ai Chao, which translates to "Super AI".
>
>>> Before: hid (null): reading report descriptor failed
>>> After: i2c_hid i2c-TPD0001:00: reading report descriptor failed
>>>
>>> Signed-off-by: Ai Chao <aichao@kylinos.cn>
>>> ---
>>> change for v2
>>> - Used Fixes tag.
>
>> Could have fooled me. ;-)
>
> syslog:
> kernel: [ 3.815131][ 1] [ T319] i2c_designware HYGO0010:00: timeout in disabling adapter
> kernel: [ 15.790171][ 1] [ T319] i2c_designware HYGO0010:00: timeout waiting for bus ready
> kernel: [ 15.998769][ 1] [ T319] i2c reading report descriptor failed retry:49
> kernel: [ 16.038176][ 1] [ T319] i2c_designware HYGO0010:00: timeout waiting for bus ready
> kernel: [ 16.246771][ 1] [ T319] i2c reading report descriptor failed retry:50
> kernel: [ 16.246775][ 1] [ T319] hid (null): reading report descriptor failed
> kernel: [ 16.246778][ 1] [ T319] i2c_hid i2c-TPD0001:00: can't add hid device: -5
> kernel: [ 16.246954][ 1] [ T319] i2c_hid: probe of i2c-TPD0001:00 failed with error -5
>
>
> Thanks,
> Ai Chao
>
>
The point was that there is no "Fixes" tag added above your SoB, but you
mentioned that there was one in your changelist. Identify the commit
that you're fixing (use git blame for example) and then add it to the
commit text above your SoB in the correct format.
# git log -1 --format='Fixes: %h ("%s")' $commit
^ permalink raw reply
* [PATCH v3] HID: i2c-hid: Avoid "(null)" output when reading report descriptor fails
From: Ai Chao @ 2026-07-10 6:35 UTC (permalink / raw)
To: jikos, bentiss, dianders, treapking, kenkinming2002, raoxu,
superm1
Cc: linux-input, linux-kernel, Ai Chao
When i2c-hid fails to read the HID report descriptor during device
initialization, the error message prints as:
hid (null): reading report descriptor failed
The HID device name is set in hid_add_device() after calling
hdev->ll_driver->parse(), so when i2c_hid_parse() fails and calls
hid_err(), the device name has not been set yet, resulting in "(null)"
output.
Use dev_err(&client->dev, ...) instead of hid_err(hid, ...) because
the I2C client device is fully initialized with a proper name, providing
meaningful error messages for debugging.
Before: hid (null): reading report descriptor failed
After: i2c_hid i2c-TPD0001:00: reading report descriptor failed
Signed-off-by: Ai Chao <aichao@kylinos.cn>
---
change for v3
- Update the patch subject.
---
drivers/hid/i2c-hid/i2c-hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..0e725a0f0abe 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -792,7 +792,7 @@ static int i2c_hid_parse(struct hid_device *hid)
ihid->hdesc.wReportDescRegister,
rdesc, rsize);
if (ret) {
- hid_err(hid, "reading report descriptor failed\n");
+ dev_err(&client->dev, "reading report descriptor failed\n");
goto out;
}
}
--
2.34.1
^ permalink raw reply related
* [PATCH] dt-bindings: input: fix bracket
From: Manuel Ebner @ 2026-07-10 9:01 UTC (permalink / raw)
To: Hugo Villeneuve, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Manuel Ebner, linux-input, devicetree, linux-kernel
Add missing '('.
Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
---
.../devicetree/bindings/input/gpio-charlieplex-keypad.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
index c085de6dab85..c6842c017934 100644
--- a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
@@ -11,7 +11,7 @@ maintainers:
- Hugo Villeneuve <hvilleneuve@dimonoff.com>
description: |
- The charlieplex keypad supports N^2)-N different key combinations (where N is
+ The charlieplex keypad supports (N^2)-N different key combinations (where N is
the number of I/O lines). Key presses and releases are detected by configuring
only one line as output at a time, and reading other line states. This process
is repeated for each line. Diodes are required to ensure current flows in only
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox