* Re: [PATCH v3 1/2] HID: sensor: custom: Fix use-after-free in enable_sensor
From: sashiko-bot @ 2026-07-07 7:50 UTC (permalink / raw)
To: Haoxiang Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707071545.3087073-2-haoxiang_li2024@163.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Incorrect mapping between the sysfs attribute name index and the `sensor_inst->fields` array leads to Heap Out-Of-Bounds reads.
- [High] The `/dev/HID-SENSOR-*` character device operations do not synchronize with device removal, leading to Use-After-Free on `devm`-managed structures.
- [High] The `data_fifo` is freed before the sensor hub interrupt callback is unregistered during device removal, creating a race condition that leads to a Use-After-Free.
- [High] The sensor hub device is never closed on driver removal if it was concurrently enabled, causing a permanent power and reference count leak.
--
--- Patch [1]: [PATCH v3 1/2] HID: sensor: custom: Fix use-after-free in enable_sensor ---
commit d433ad359db5f5f5744d4b3f8a4dc8088af760b6
Author: Haoxiang Li <haoxiang_li2024@163.com>
HID: sensor: custom: Fix use-after-free in enable_sensor
This commit fixes a use-after-free by altering the initialization and cleanup
order of the sysfs attributes and the enable_sensor group. This prevents
enable_sensor from being accessed before its dependencies are fully
initialized or after they have been freed.
Link: https://sashiko.dev/#/patchset/20260623021950.1736413-1-haoxiang_li2024@163.com?part=1
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index afffea8940214..6b0da2e0e1c93 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -1005,26 +1005,26 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
> - &enable_sensor_attr_group);
> + ret = hid_sensor_custom_add_attributes(sensor_inst);
[Severity: Critical]
This is a pre-existing issue, but is there an out-of-bounds read when parsing
the sysfs strings created by this function?
During initialization, if a HID field is skipped, it is not appended to the
sensor_inst->fields array, keeping sensor_field_count small. However, its
original field index is embedded into the sysfs file name.
When a valid sysfs file is later read, show_value() parses the index from
the filename and uses it directly as an array index:
if (sscanf(attr->attr.name, "input-%x-%x-%s",
&index, &usage, name) == 3) {
input = true;
field_index = index;
}
...
value = sensor_inst->fields[field_index].attribute.logical_minimum;
Could this bypass the array bounds and leak memory to userspace?
[ ... ]
> @@ -1042,9 +1042,10 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
> }
>
> hid_sensor_custom_dev_if_remove(sensor_inst);
[Severity: High]
This isn't a bug introduced by this patch, but doesn't this leave a
use-after-free window?
If a user holds open the misc device /dev/HID-SENSOR-*, misc_deregister()
does not close existing open file descriptors. Since sensor_inst is
allocated with devm_kzalloc(), it is freed when remove() returns.
Would subsequent calls to hid_sensor_custom_read(), poll(), or
hid_sensor_custom_release() dereference the freed sensor_inst object?
[Severity: High]
This is a pre-existing issue, but is there a race condition here?
The data_fifo is freed by hid_sensor_custom_dev_if_remove() before the
interrupt callback is unregistered by sensor_hub_remove_callback().
If an interrupt occurs while sysfs_remove_group() or
hid_sensor_custom_remove_attributes() is blocking, couldn't the callback
hid_sensor_capture_sample() write incoming data to the freed data_fifo
via kfifo_in()?
[Severity: High]
This is also a pre-existing issue, but does this function leak the sensor hub
reference and power state?
If a user writes '1' to the enable_sensor sysfs attribute,
sensor_hub_device_open() powers on the device and takes a reference. Since
this function tears down the interfaces without checking sensor_inst->enable
and calling sensor_hub_device_close(), does the HID device reference leak
permanently?
> - hid_sensor_custom_remove_attributes(sensor_inst);
> + /* Remove enable_sensor first as it uses fields via power_state/report_state. */
> sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> &enable_sensor_attr_group);
> + hid_sensor_custom_remove_attributes(sensor_inst);
> sensor_hub_remove_callback(hsdev, hsdev->usage);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707071545.3087073-1-haoxiang_li2024@163.com?part=1
^ permalink raw reply
* Re: [PATCH v3 2/2] HID: sensor: custom: Fix field sysfs group cleanup on failure
From: sashiko-bot @ 2026-07-07 8:04 UTC (permalink / raw)
To: Haoxiang Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707071545.3087073-3-haoxiang_li2024@163.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Lifetime mismatch between `devm_kzalloc` allocated `sensor_inst` and `miscdevice` file operations leads to a Use-After-Free.
- [High] Race condition in `hid_sensor_custom_remove()` between event callback removal and kfifo destruction allows Use-After-Free.
--
--- Patch [2]: [PATCH v3 2/2] HID: sensor: custom: Fix field sysfs group cleanup on failure ---
commit aa8aaa0518fc58fa29db9643742276c6a56cafe6
Author: Haoxiang Li <haoxiang_li2024@163.com>
HID: sensor: custom: Fix field sysfs group cleanup on failure
hid_sensor_custom_add_attributes() creates one sysfs group for each
custom sensor field. If sysfs_create_group() fails after some groups
have already been created, the function returns the error without
removing the previously created groups.
Add a local unwind path to remove the groups that were already created.
With enable_sensor exposed only after the field attributes are ready,
this path can free sensor_inst->fields without leaving enable_sensor
able to access pointers into that array.
Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index 6b0da2e0e1c93..c2b425afd951b 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -609,7 +609,7 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
[ ... ]
> return ret;
> +
> +err_remove_groups:
> + while (--i >= 0)
> + sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> + &sensor_inst->fields[i].hid_custom_attribute_group);
> + kfree(sensor_inst->fields);
> + return ret;
> }
>
> static void hid_sensor_custom_remove_attributes(struct hid_sensor_custom *
[Severity: High]
This is a pre-existing issue, but is there a lifetime mismatch between the
devm_kzalloc() allocated sensor_inst and the miscdevice file operations?
In hid_sensor_custom_probe(), sensor_inst is allocated with device-managed
memory:
sensor_inst = devm_kzalloc(&pdev->dev, sizeof(*sensor_inst),
GFP_KERNEL);
When the device is unbound, devres will automatically free sensor_inst.
However, misc_deregister() does not revoke existing open file descriptors.
If a user has the character device open, unbinds the device, and then
interacts with the file descriptor, could this lead to a use-after-free?
For example, hid_sensor_custom_read() accesses the freed pointer:
sensor_inst = container_of(file->private_data,
struct hid_sensor_custom, custom_dev);
Similarly, closing the file calls hid_sensor_custom_release() which modifies
the freed memory:
clear_bit(0, &sensor_inst->misc_opened);
[Severity: High]
This is a pre-existing issue, but is there a race condition in
hid_sensor_custom_remove() during device teardown?
The function first frees the data buffer:
hid_sensor_custom_dev_if_remove(sensor_inst); // calls kfifo_free()
It removes the callback later in the function:
sensor_hub_remove_callback(hsdev, hsdev->usage);
If the sensor hub receives an asynchronous hardware event between these two
calls, wouldn't the still-active callback invoke hid_sensor_capture_sample()
and write incoming hardware data to the freed data_fifo?
kfifo_in(&sensor_inst->data_fifo, (unsigned char *)raw_data,
raw_len);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707071545.3087073-1-haoxiang_li2024@163.com?part=2
^ permalink raw reply
* Re: [PATCH v2] HID: hid-lenovo-go: cancel cfg_setup work in hid_go_cfg_remove()
From: Günther Noack @ 2026-07-07 9:06 UTC (permalink / raw)
To: Derek J. Clark
Cc: Manish Khadka, dmitry.torokhov, linux-input, sashiko-reviews,
Jiri Kosina, Benjamin Tissoires, Lee Jones, Mark Pearson
In-Reply-To: <20260515182347.6C82FC2BCB0@smtp.kernel.org>
Hello Derek and Mark!
May I please draw your attention to this issue in the hid-lenovo drivers,
which was pointed out by Sashiko:
On Fri, May 15, 2026 at 06:23:46PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
> - [Critical] Hardware state corruption and crashes due to use of a static global struct for device data.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> > index d4d26c7833563..b9d8cde531365 100644
> > --- a/drivers/hid/hid-lenovo-go.c
> > +++ b/drivers/hid/hid-lenovo-go.c
> > @@ -2408,6 +2408,15 @@ static int hid_go_cfg_probe(struct hid_device *hdev,
> >
> > static void hid_go_cfg_remove(struct hid_device *hdev)
> > {
> > + /*
> > + * cfg_setup is scheduled from hid_go_cfg_probe() with a 2 ms delay
> > + * and dereferences drvdata.hdev. Drain it here before tearing
> > + * down so the workqueue cannot run after hid_destroy_device()'s
> > + * put_device() has released the underlying hdev and dereference
> > + * a stale drvdata.hdev pointer.
> > + */
> > + cancel_delayed_work_sync(&drvdata.go_cfg_setup);
>
> This isn't a bug introduced by this patch, but does the driver use a global
> static variable (static struct hid_go_cfg drvdata) to store device state?
>
> A second device probe would unconditionally overwrite drvdata.hdev and
> re-initialize both drvdata.cfg_mutex and drvdata.go_cfg_setup.
>
> Re-initializing an active mutex destroys its wait lists, and re-initializing
> an active delayed work overwrites its linked list pointers, leading to
> immediate memory corruption and kernel panics.
>
> Should the device state be dynamically allocated per-instance instead?
This is the same issue which was also introduced in hid-oxp, which I
pointed out in [1]. As the review-bot points out above, while the data in
your static drvdata variable is maybe only necessary once (but that
also seems questionable?), I think the mutex and workqueue shouldn't
just get re-initialized in the middle of being used, when a second device is
plugged in and the probe() hook runs again.
This pattern is used in both the drivers hid-lenovo-go.c, hid-lenovo-go-s.c,
as well as in the newer hid-oxp discussed in [1].
Shouldn't the drvdata just be allocated with one of the devm_*alloc() variants
as in the other drivers?
—Günther
[1] https://lore.kernel.org/all/akfGTjId84EjV141@google.com/
^ permalink raw reply
* [PATCH 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
From: Lee Jones @ 2026-07-07 12:10 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Peter Hutterer, Dmitry Torokhov, linux-input,
linux-kernel
Input subsystem guidelines require that device capabilities are advertised
before the input device is registered. The Wacom driver was violating
this by advertising the SW_MUTE_DEVICE capability post-registration in
wacom_set_shared_values() (and duplicating it in device-specific setup
cases).
Resolve this by moving the SW_MUTE_DEVICE capability setup to
wacom_setup_touch_input_capabilities() for all touch devices that support
it, excluding TabletPC devices.
Additionally, replace the lookup-dependent
'wacom_wac->shared->touch->product' references with 'hdev->product'
inside wacom_setup_touch_input_capabilities() as 'hdev' is already
available and represents the touch device itself.
Fixes: d2ec58aee8b1 ("HID: wacom: generic: support generic touch switch")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v4 -> v5: New patch used to split out SW_MUTE_DEVICE as per Jason's request
drivers/hid/wacom_sys.c | 6 ------
drivers/hid/wacom_wac.c | 21 +++++++++++++--------
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..60b380efdfb7 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2361,12 +2361,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
wacom_wac->shared->is_touch_on = true;
}
- if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
- }
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..a31325be50f8 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3951,6 +3951,8 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ struct hid_device *hdev = wacom->hdev;
struct wacom_features *features = &wacom_wac->features;
if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
@@ -3961,9 +3963,14 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
else
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
- if (features->type == HID_GENERIC)
+ if (features->type == HID_GENERIC) {
+ if (wacom_wac->shared && wacom_wac->shared->has_mute_touch_switch) {
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ }
/* setup has already been done */
return 0;
+ }
input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
__set_bit(BTN_TOUCH, input_dev->keybit);
@@ -3995,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
input_dev->evbit[0] |= BIT_MASK(EV_SW);
__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
- if (wacom_wac->shared->touch->product == 0x361) {
+ if (hdev->product == 0x361) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 12440, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 8640, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x360) {
+ } else if (hdev->product == 0x360) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 8960, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 5920, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x393) {
+ } else if (hdev->product == 0x393) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 6400, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -4037,8 +4042,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
fallthrough;
case WACOM_27QHDT:
- if (wacom_wac->shared->touch->product == 0x32C ||
- wacom_wac->shared->touch->product == 0xF6) {
+ if (hdev->product == 0x32C ||
+ hdev->product == 0xF6) {
input_dev->evbit[0] |= BIT_MASK(EV_SW);
__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
wacom_wac->has_mute_touch_switch = true;
--
2.55.0.rc2.803.g1fd1e6609c-goog
^ permalink raw reply related
* [PATCH 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-07-07 12:10 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <20260707121051.1683937-1-lee@kernel.org>
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
inside the interrupt handler context. If the Touch sibling device
is disconnected, wacom_remove_shared_data() clears 'touch_input'
outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
race condition where a preempted reader in interrupt context
dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input
pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap all lockless readers in wacom_wac.c with guard(rcu)() and
rcu_dereference() using a unified wacom_report_touch_mute()
helper
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure
all active RCU readers have finished before the input device is
freed
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
concurrent probe assignments, and verify that 'shared->touch == hdev'
before setting touch_input to prevent concurrent sibling probe state
desynchronization.
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [redundant check and guard()]
v4 -> v5: Jason's review [split, remove "awkward if"]
drivers/hid/wacom_sys.c | 35 +++++++++++++++++++++++------------
drivers/hid/wacom_wac.c | 36 ++++++++++++++++++------------------
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 60b380efdfb7..86895f13dbae 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
- if (wacom_wac->shared->touch == wacom->hdev)
- wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
- wacom_wac->shared->pen = NULL;
+ scoped_guard(mutex, &wacom_udev_list_lock) {
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->touch = NULL;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
+ wacom_wac->shared->pen = NULL;
+ }
+ }
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (retval)
return retval;
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ guard(mutex)(&wacom_udev_list_lock);
+
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2360,7 +2372,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
if (wacom_wac->is_soft_touch_switch)
wacom_wac->shared->is_touch_on = true;
}
-
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a31325be50f8..f3429a2c8f36 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -510,6 +510,18 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
}
}
+static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
+{
+ struct input_dev *touch_input;
+
+ guard(rcu)();
+ touch_input = rcu_dereference(wacom_wac->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
+ input_sync(touch_input);
+ }
+}
+
static int wacom_intuos_pad(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
@@ -650,12 +662,8 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
- }
+ if (wacom->shared)
+ wacom_report_touch_mute(wacom, !wacom->shared->is_touch_on);
input_report_abs(input, ABS_RX, strip1);
input_report_abs(input, ABS_RY, strip2);
@@ -2153,7 +2161,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
*/
if ((equivalent_usage == WACOM_HID_WD_MUTE_DEVICE) ||
(equivalent_usage == WACOM_HID_WD_TOUCHONOFF)) {
- if (wacom_wac->shared->touch_input) {
+ if (wacom_wac->shared) {
bool *is_touch_on = &wacom_wac->shared->is_touch_on;
if (equivalent_usage == WACOM_HID_WD_MUTE_DEVICE && value)
@@ -2161,9 +2169,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
else if (equivalent_usage == WACOM_HID_WD_TOUCHONOFF)
*is_touch_on = value;
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, !(*is_touch_on));
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, !(*is_touch_on));
}
return;
}
@@ -3381,11 +3387,8 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
if ((wacom->shared->type == INTUOSHT ||
wacom->shared->type == INTUOSHT2) &&
- wacom->shared->touch_input &&
wacom->shared->touch_max) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE, data[5] & 0x40);
- input_sync(wacom->shared->touch_input);
+ wacom_report_touch_mute(wacom, data[5] & 0x40);
}
pid = get_unaligned_be16(&data[6]);
@@ -3420,11 +3423,8 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
if ((features->type == INTUOSHT ||
features->type == INTUOSHT2) &&
- wacom_wac->shared->touch_input &&
features->touch_max) {
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, data[8] & 0x40);
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, data[8] & 0x40);
}
if (data[9] & 0x02) { /* wireless module is attached */
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.55.0.rc2.803.g1fd1e6609c-goog
^ permalink raw reply related
* [PATCH 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: Lee Jones @ 2026-07-07 12:10 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Peter Hutterer, Dmitry Torokhov, linux-input,
linux-kernel
In-Reply-To: <20260707121051.1683937-1-lee@kernel.org>
wacom_bamboo_pad_pen_event() accesses wacom->shared->pen locklessly
relative to wacom_remove_shared_data() which nullifies it. This
can lead to a Use-After-Free if the sibling device is removed while
events are being processed.
Resolve this by introducing RCU protection for pen and touch
pointers:
- Annotate 'pen' and 'touch' in wacom_shared struct with __rcu.
- Wrap lockless readers in wacom_bamboo_pad_pen_event() with
rcu_read_lock() and rcu_dereference().
- Update writers in wacom_sys.c using rcu_assign_pointer().
- Use rcu_dereference_protected for comparisons under
wacom_udev_list_lock.
- Also use rcu_access_pointer in wacom_mode_change_work() to avoid
warnings (while lockless access there remains a pre-existing issue).
Fixes: 8c97a765467c ("HID: wacom: add full support of the Wacom Bamboo PAD")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [guard()]
v4 -> v5: No change
drivers/hid/wacom_sys.c | 36 +++++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 13 ++++++-------
drivers/hid/wacom_wac.h | 4 ++--
3 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 86895f13dbae..3990d8d0b40c 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -878,11 +878,18 @@ static void wacom_remove_shared_data(void *res)
shared);
scoped_guard(mutex, &wacom_udev_list_lock) {
- if (wacom_wac->shared->touch == wacom->hdev) {
- wacom_wac->shared->touch = NULL;
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+ struct hid_device *pen =
+ rcu_dereference_protected(wacom_wac->shared->pen,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
- wacom_wac->shared->pen = NULL;
+ } else if (pen == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
@@ -916,9 +923,9 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- data->shared.touch = hdev;
+ rcu_assign_pointer(data->shared.touch, hdev);
else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- data->shared.pen = hdev;
+ rcu_assign_pointer(data->shared.pen, hdev);
mutex_unlock(&wacom_udev_list_lock);
@@ -2356,7 +2363,11 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
guard(mutex)(&wacom_udev_list_lock);
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- if (wacom_wac->shared->touch == wacom->hdev) {
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
wacom_wac->shared->type = wacom_wac->features.type;
rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
}
@@ -2795,16 +2806,19 @@ static void wacom_mode_change_work(struct work_struct *work)
bool is_direct = wacom->wacom_wac.is_direct_mode;
int error = 0;
- if (shared->pen) {
- wacom1 = hid_get_drvdata(shared->pen);
+ struct hid_device *pen = rcu_access_pointer(shared->pen);
+ struct hid_device *touch = rcu_access_pointer(shared->touch);
+
+ if (pen) {
+ wacom1 = hid_get_drvdata(pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
wacom1->wacom_wac.has_mode_change = true;
wacom1->wacom_wac.is_direct_mode = is_direct;
}
- if (shared->touch) {
- wacom2 = hid_get_drvdata(shared->touch);
+ if (touch) {
+ wacom2 = hid_get_drvdata(touch);
wacom_release_resources(wacom2);
hid_hw_stop(wacom2->hdev);
wacom2->wacom_wac.has_mode_change = true;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index f3429a2c8f36..8581c33fac59 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3292,6 +3292,7 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
unsigned char *data)
{
+ struct hid_device *pen;
unsigned char prefix;
/*
@@ -3304,13 +3305,11 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
prefix = data[0];
data[0] = WACOM_REPORT_BPAD_PEN;
- /*
- * actually reroute the event.
- * No need to check if wacom->shared->pen is valid, hid_input_report()
- * will check for us.
- */
- hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
- WACOM_PKGLEN_PENABLED, 1);
+ guard(rcu)();
+ pen = rcu_dereference(wacom->shared->pen);
+ if (pen)
+ hid_input_report(pen, HID_INPUT_REPORT, data,
+ WACOM_PKGLEN_PENABLED, 1);
data[0] = prefix;
}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index a8bbba4a6f37..170d6adbe02a 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -286,8 +286,8 @@ struct wacom_shared {
unsigned touch_max;
int type;
struct input_dev __rcu *touch_input;
- struct hid_device *pen;
- struct hid_device *touch;
+ struct hid_device __rcu *pen;
+ struct hid_device __rcu *touch;
bool has_mute_touch_switch;
bool is_touch_on;
};
--
2.55.0.rc2.803.g1fd1e6609c-goog
^ permalink raw reply related
* [PATCH 4/4] HID: wacom: Redesign shared sibling data lifecycle
From: Lee Jones @ 2026-07-07 12:10 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <20260707121051.1683937-1-lee@kernel.org>
The Wacom driver coordinates state between sibling interfaces of
the same physical device using a shared structure 'wacom_shared'
inside 'wacom_hdev_data'. The driver kept a volatile representative
pointer 'data->dev' pointing to a sibling 'hid_device' for physical
path comparisons during sibling matching.
This pointer management is fragile. When the representative device
is disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when
subsequent sibling probes dereference the dangling 'data->dev'
pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer
completely
- Redesign 'wacom_hdev_data' to store stable static copies of the
required attributes upon first allocation: 'phys' path string,
'vendor', 'product' IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if
individual siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in
wacom_remove_shared_data() to serialize refcount drops with list
traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is
already held
Also, do not accumulate the 'device_type' capability flag during
subsequent sibling probes. Keeping only the first probed sibling's
device_type exactly preserves the original sibling matching behavior
without introducing side effects.
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: No change
v4 -> v5: No change
drivers/hid/wacom_sys.c | 70 +++++++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 3990d8d0b40c..9fb9dde7dd73 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,47 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ const char *p1 = strrchr(hdev_a->phys, separator);
+ const char *p2 = strrchr(phys_b, separator);
+ int n1, n2;
+
+ if (!p1 || !p2)
+ return false;
+
+ n1 = p1 - hdev_a->phys;
+ n2 = p2 - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -787,11 +807,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -804,7 +824,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -812,17 +832,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -838,7 +858,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -846,7 +866,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -860,18 +880,15 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
@@ -885,17 +902,19 @@ static void wacom_remove_shared_data(void *res)
rcu_dereference_protected(wacom_wac->shared->pen,
lockdep_is_held(&wacom_udev_list_lock));
- if (touch == wacom->hdev) {
+ if (touch == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (pen == wacom->hdev) {
+ } else if (pen == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -918,7 +937,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strscpy(data->phys, hdev->phys, sizeof(data->phys));
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.55.0.rc2.803.g1fd1e6609c-goog
^ permalink raw reply related
* Re: [PATCH 4/4] HID: wacom: Redesign shared sibling data lifecycle
From: sashiko-bot @ 2026-07-07 12:25 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707121051.1683937-4-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent sibling unbind causes Use-After-Free in wacom_mode_change_work()
--
--- Patch [4]: [PATCH 4/4] HID: wacom: Redesign shared sibling data lifecycle ---
commit 5aab8fc5d0e17a15712b387b6273c3b728adb79d
Author: Lee Jones <lee@kernel.org>
HID: wacom: Redesign shared sibling data lifecycle
The Wacom driver coordinates state between sibling interfaces of the same
physical device using a shared structure 'wacom_shared' inside
'wacom_hdev_data'. This patch redesigns the sibling data lifecycle to use
stable static attributes for sibling matching and serializes refcount drops
in wacom_remove_shared_data().
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -883,17 +900,19 @@ static void wacom_remove_shared_data(void *res)
> rcu_dereference_protected(wacom_wac->shared->pen,
> lockdep_is_held(&wacom_udev_list_lock));
>
> - if (touch == wacom->hdev) {
> + if (touch == res_wacom->hdev) {
> rcu_assign_pointer(wacom_wac->shared->touch, NULL);
> rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> - } else if (pen == wacom->hdev) {
> + } else if (pen == res_wacom->hdev) {
> rcu_assign_pointer(wacom_wac->shared->pen, NULL);
> }
> }
>
> synchronize_rcu();
[Severity: High]
This is a pre-existing issue, but while reviewing these RCU lifecycle changes,
I noticed a potential use-after-free condition involving the pointers being
managed here.
When a sibling interface is concurrently unbound, wacom_remove_shared_data()
clears the pointer and calls synchronize_rcu() here.
However, looking at wacom_mode_change_work() in drivers/hid/wacom_sys.c, the
shared sibling pointers are accessed without holding an RCU read lock:
struct hid_device *touch = rcu_access_pointer(shared->touch);
if (touch) {
wacom2 = hid_get_drvdata(touch);
wacom_release_resources(wacom2);
hid_hw_stop(wacom2->hdev);
}
Because wacom_mode_change_work() does not enter an RCU read-side critical
section, synchronize_rcu() won't wait for it to finish before returning
and allowing the sibling device to be freed.
Could this sequence lead to a use-after-free if wacom_mode_change_work()
subsequently dereferences the touch device pointer after it has been freed
by the driver core?
> - kref_put(&data->kref, wacom_release_shared_data);
> + if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
> + mutex_unlock(&wacom_udev_list_lock);
> +
> wacom_wac->shared = NULL;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707121051.1683937-1-lee@kernel.org?part=4
^ permalink raw reply
* Re: [PATCH 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: sashiko-bot @ 2026-07-07 12:26 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260707121051.1683937-2-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Pre-existing use-after-free race between kref_put() in wacom_remove_shared_data() and list traversal in wacom_get_hdev_data().
- [High] Lockless reads of `shared->pen` and `shared->touch` lead to TOCTOU Use-After-Free.
--
--- Patch [2]: [PATCH 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad ---
commit 7367a08b48f1d1fc2e7dc8823f95547c3c03eb51
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
inside the interrupt handler context. If the Touch sibling device
is disconnected, wacom_remove_shared_data() clears touch_input
outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
race condition.
Resolve this by introducing RCU protection for the touch_input
pointer.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 6c2a3ddfe366a..c6b37931d02d1 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -875,10 +875,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> - wacom_wac->shared->touch = NULL;
> - else if (wacom_wac->shared->pen == wacom->hdev)
> - wacom_wac->shared->pen = NULL;
> + scoped_guard(mutex, &wacom_udev_list_lock) {
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->touch = NULL;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> + } else if (wacom_wac->shared->pen == wacom->hdev) {
> + wacom_wac->shared->pen = NULL;
> + }
> + }
[Severity: High]
This isn't a bug introduced by this patch, but since we are introducing
RCU protection for touch_input, do shared->pen and shared->touch need
similar RCU annotations and read guards?
wacom_mode_change_work() reads both pointers locklessly in preemptible
workqueue context:
drivers/hid/wacom_sys.c:wacom_mode_change_work() {
...
if (shared->pen) {
wacom1 = hid_get_drvdata(shared->pen);
...
if (shared->touch) {
wacom2 = hid_get_drvdata(shared->touch);
...
}
Similarly, wacom_bamboo_pad_pen_event() dereferences shared->pen without
protection:
drivers/hid/wacom_wac.c:wacom_bamboo_pad_pen_event() {
...
hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data, ...);
...
}
Because these readers run without rcu_read_lock() or guard(rcu), the new
synchronize_rcu() call won't wait for them, potentially leaving them to
dereference freed pointers when the sibling device unbinds. Should pen and
touch also be converted to RCU pointers?
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
[Severity: High]
This is a pre-existing issue, but does this lockless kref_put() create a
race condition with concurrent device probes?
If a concurrent unbind and probe occurs, kref_put() can drop the refcount
to 0 and synchronously call wacom_release_shared_data(). Before the
release function can acquire the wacom_udev_list_lock to remove the
object, a concurrent wacom_add_shared_data() might call
wacom_get_hdev_data().
wacom_get_hdev_data() iterates the list under the lock, finds the still-
linked object, and blindly calls kref_get(&data->kref) on the 0-refcount
object:
drivers/hid/wacom_sys.c:wacom_get_hdev_data() {
...
list_for_each_entry(data, &wacom_udev_list, list) {
if (strncmp(data->phys, hdev->phys, sizeof(data->phys) - 2) == 0) {
kref_get(&data->kref);
return data;
}
...
}
When the lock is dropped, the release function proceeds to kfree() the
object, leaving the prober with a freed pointer. Should this be using
kref_put_mutex() or kref_get_unless_zero() to safely handle the list
removal?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707121051.1683937-1-lee@kernel.org?part=2
^ permalink raw reply
* Re: [PATCH 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: sashiko-bot @ 2026-07-07 12:27 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260707121051.1683937-3-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless access to `shared->pen` and `shared->touch` in `wacom_mode_change_work()` leads to Use-After-Free if a sibling device is disconnected.
- [High] Race condition between `kref_put()` and `kref_get()` for `wacom_hdev_data` leads to Use-After-Free.
--
--- Patch [3]: [PATCH 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad ---
commit ba24ac0ee968766a5b0db4400d76393921713f71
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
This commit introduces RCU protection for pen and touch pointers in the
wacom_shared struct to resolve a use-after-free issue. It wraps lockless
readers with RCU read locks and updates writers to use RCU assignment and
dereferencing functions.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index c6b37931d02d1..5578de80f4c80 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -876,11 +876,18 @@ static void wacom_remove_shared_data(void *res)
> shared);
>
> scoped_guard(mutex, &wacom_udev_list_lock) {
> - if (wacom_wac->shared->touch == wacom->hdev) {
> - wacom_wac->shared->touch = NULL;
> + struct hid_device *touch =
> + rcu_dereference_protected(wacom_wac->shared->touch,
> + lockdep_is_held(&wacom_udev_list_lock));
> + struct hid_device *pen =
> + rcu_dereference_protected(wacom_wac->shared->pen,
> + lockdep_is_held(&wacom_udev_list_lock));
> +
> + if (touch == wacom->hdev) {
> + rcu_assign_pointer(wacom_wac->shared->touch, NULL);
> rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> - } else if (wacom_wac->shared->pen == wacom->hdev) {
> - wacom_wac->shared->pen = NULL;
> + } else if (pen == wacom->hdev) {
> + rcu_assign_pointer(wacom_wac->shared->pen, NULL);
> }
> }
>
[Severity: High]
This is a pre-existing issue, but is there a race condition between
kref_put() and kref_get() for wacom_hdev_data that could lead to a
use-after-free?
In wacom_remove_shared_data(), kref_put() is called without holding
wacom_udev_list_lock:
kref_put(&data->kref, wacom_release_shared_data);
If the refcount drops to 0, it calls wacom_release_shared_data(), which
acquires the lock to remove the element.
In the window before the lock is acquired, could another thread in
wacom_add_shared_data() iterate over wacom_udev_list, find the still-linked
data, and call kref_get(&data->kref)?
This would increment a 0-refcount object, and subsequently the first thread
frees it, leaving the second thread with a dangling pointer.
[ ... ]
> @@ -2793,16 +2804,19 @@ static void wacom_mode_change_work(struct work_struct *work)
> bool is_direct = wacom->wacom_wac.is_direct_mode;
> int error = 0;
>
> - if (shared->pen) {
> - wacom1 = hid_get_drvdata(shared->pen);
> + struct hid_device *pen = rcu_access_pointer(shared->pen);
> + struct hid_device *touch = rcu_access_pointer(shared->touch);
> +
> + if (pen) {
> + wacom1 = hid_get_drvdata(pen);
[Severity: High]
This is a pre-existing issue, but does lockless access to shared->pen and
shared->touch in wacom_mode_change_work() lead to a use-after-free if a
sibling device is disconnected?
Pointers are fetched locklessly via rcu_access_pointer() and subsequently
dereferenced without any synchronization. If a sibling device is disconnected,
wacom_remove_shared_data() nullifies the pointer and calls synchronize_rcu().
Because wacom_mode_change_work() runs asynchronously and reads the pointer
without holding rcu_read_lock() or wacom_udev_list_lock, synchronize_rcu()
does not wait for it.
Can this cause the sibling device's memory to be freed and subsequently
dereferenced by the worker thread (e.g., via hid_get_drvdata(pen))?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707121051.1683937-1-lee@kernel.org?part=3
^ permalink raw reply
* Re: [PATCH 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
From: sashiko-bot @ 2026-07-07 12:28 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260707121051.1683937-1-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the `SW_MUTE_DEVICE` capability assignment into `wacom_setup_touch_input_capabilities()` silently breaks the mute switch feature for generic Wacom devices due to interface probe ordering.
--
--- Patch [1]: [PATCH 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration ---
commit 36a40be02315e2e469201d6490c033d8bc04fa73
Author: Lee Jones <lee@kernel.org>
HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
This commit moves the SW_MUTE_DEVICE capability advertisement into
wacom_setup_touch_input_capabilities() to ensure it is set prior to input
device registration, conforming to input subsystem guidelines. It also
removes the post-registration assignment in wacom_set_shared_values() and
simplifies some product ID checks.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..6c2a3ddfe366a 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2359,12 +2359,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> wacom_wac->shared->is_touch_on = true;
> }
>
> - if (wacom_wac->shared->has_mute_touch_switch &&
> - wacom_wac->shared->touch_input) {
> - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> - SW_MUTE_DEVICE);
> - }
> }
[ ... ]
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..a31325be50f8c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3951,6 +3951,8 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> struct wacom_wac *wacom_wac)
> {
[ ... ]
> - if (features->type == HID_GENERIC)
> + if (features->type == HID_GENERIC) {
> + if (wacom_wac->shared && wacom_wac->shared->has_mute_touch_switch) {
> + input_dev->evbit[0] |= BIT_MASK(EV_SW);
> + __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> + }
> /* setup has already been done */
> return 0;
> + }
[Severity: High]
Does moving this assignment inadvertently break the mute switch on composite
USB devices?
For multi-interface generic Wacom devices, the touch interface typically
probes before the pad interface. During the touch interface's initialization,
this checks wacom_wac->shared->has_mute_touch_switch.
Since the pad interface (which contains the mute switch usage) hasn't been
parsed yet, has_mute_touch_switch will still be false.
Because the fallback assignment in wacom_set_shared_values() was removed
above, will this result in the SW_MUTE_DEVICE capability never being
advertised to the touch device?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707121051.1683937-1-lee@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH 08/10] HID: apple: Add DockChannel HID transport driver
From: Michael Reeves @ 2026-07-07 13:37 UTC (permalink / raw)
To: Julian Braha
Cc: Sven Peter, Janne Grunau, Neal Gompa, Jassi Brar, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hector Martin,
Joerg Roedel (AMD), Will Deacon, Robin Murphy, Dmitry Torokhov,
Jiri Kosina, Benjamin Tissoires, asahi, linux-arm-kernel,
linux-kernel, devicetree, iommu, linux-input
In-Reply-To: <a0cee001-7bca-438a-943b-da28528e7df7@gmail.com>
Hi Julian,
[...]
> APPLE_DOCKCHANNEL_HID has a duplicate dependency on HID,
> since you put the import for this file inside of 'if HID..endif',
> and then also gave it a 'depends on HID'.
>
Thank you for picking it up and letting me know!
I will correct it in v2, which I am planning on sending later this week, or
potentially on the weekend if I get busy.
Thanks again,
Michael
^ permalink raw reply
* Re: [PATCH 05/10] mailbox: apple: Add DockChannel FIFO controller
From: Michael Reeves @ 2026-07-07 13:42 UTC (permalink / raw)
To: Yureka Lilian
Cc: Sven Peter, Janne Grunau, Neal Gompa, Jassi Brar, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hector Martin,
Joerg Roedel (AMD), Will Deacon, Robin Murphy, Dmitry Torokhov,
Jiri Kosina, Benjamin Tissoires, asahi, linux-arm-kernel,
linux-kernel, devicetree, iommu, linux-input
In-Reply-To: <e23e4581-7eee-4f4e-aa82-6b6bb6177678@cyberchaos.dev>
On Sun, Jul 5, 2026 at 6:14 AM Yureka Lilian <yureka@cyberchaos.dev> wrote:
[...]
> > + while (block) {
> > + writeb_relaxed(*p++, dc->data_base + DATA_TX8);
>
> This must be writel_relaxed, as writeb_relaxed in this mmio region will
> cause an SError. This is why we have separate addresses putting 8-bits
> and 32-bits into the FIFO I guess.
Ah, thanks for the pickup. Sorry for the silly typo from me, must have missed
it because all the HID writes are 4-byte aligned. Will correct in v2
which I should
be able to send out by the weekend unless I get busy.
Thanks again,
Michael
^ permalink raw reply
* [PATCH RFC 0/2] HID: Replace system_wq with system_dfl_wq
From: Marco Crivellari @ 2026-07-07 14:53 UTC (permalink / raw)
To: linux-kernel, linux-input
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Jiri Kosina, Benjamin Tissoires, Derek J. Clark
Hi,
Currently the code uses the per-cpu workqueue system_wq to schedule
delayed works.
Unbound works could benefit from scheduler task placement, to optimize
performance and power consumption. Another good reason to have this unbound,
is the "queue_delayed_work()" function, used to enqueue the work item.
~~~ Details about queue_delayed_work ~~~
system_wq is a per-cpu workqueue and it is used as a parameter of
mod_delayed_work(). This function schedule an item that it will later
be enqueued (once the timer will fire). __queue_delayed_work() does the job
receiving as "cpu" WORK_CPU_UNBOUND:
if (housekeeping_enabled(HK_TYPE_TIMER)) {
// [....]
} else {
if (likely(cpu == WORK_CPU_UNBOUND))
add_timer_global(timer);
else
add_timer_on(timer, cpu);
}
The timer is global, so can fire everywhere, and the work item will be
enqueued where the timer fired.
Since the workqueue work doesn't rely on per-cpu variables, there is no
obvious reason that justify the use of a per-cpu workqueue. So change the
workqueue with system_dfl_wq, so that the used workqueue is now unbound
and can benefit from scheduler task placement.
Thanks!
Marco Crivellari (2):
HID: hid-oxp: Replace system_wq with system_dfl_wq
HID: appletb-kdb: Replace system_wq with system_dfl_wq
drivers/hid/hid-appletb-kbd.c | 6 +++---
drivers/hid/hid-oxp.c | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH RFC 1/2] HID: hid-oxp: Replace system_wq with system_dfl_wq
From: Marco Crivellari @ 2026-07-07 14:53 UTC (permalink / raw)
To: linux-kernel, linux-input
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Jiri Kosina, Benjamin Tissoires, Derek J. Clark
In-Reply-To: <20260707145327.313292-1-marco.crivellari@suse.com>
The function end up calling __queue_delayed_work(), which set a global
timer that could fire anywhere, enqueuing the work where the timer fired.
Unbound works could benefit from scheduler task placement, to optimize
performance and power consumption.
Since the workqueue work doesn't rely on per-cpu variables, there is no
obvious reason that justify the use of a per-cpu workqueue. So change the
workqueue with the new unbound version, system_dfl_wq.
Cc: Derek J. Clark <derekjohn.clark@gmail.com>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
drivers/hid/hid-oxp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 20a54f337220..d2ded6b08ce9 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -398,7 +398,7 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
* Re-apply our settings after this has been received.
*/
if (data[3] == OXP_EFFECT_MONO_TRUE) {
- mod_delayed_work(system_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
+ mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
return 0;
}
@@ -788,7 +788,7 @@ static ssize_t map_button_store(struct device *dev,
default:
return -EINVAL;
}
- mod_delayed_work(system_wq, &drvdata.oxp_btn_queue, msecs_to_jiffies(50));
+ mod_delayed_work(system_dfl_wq, &drvdata.oxp_btn_queue, msecs_to_jiffies(50));
return count;
}
@@ -1349,7 +1349,7 @@ static void oxp_rgb_brightness_set(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
led_cdev->brightness = brightness;
- mod_delayed_work(system_wq, &drvdata.oxp_rgb_queue, msecs_to_jiffies(50));
+ mod_delayed_work(system_dfl_wq, &drvdata.oxp_rgb_queue, msecs_to_jiffies(50));
}
static struct attribute *oxp_rgb_attrs[] = {
@@ -1502,7 +1502,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
drvdata.rumble_intensity = 5;
INIT_DELAYED_WORK(&drvdata.oxp_mcu_init, oxp_mcu_init_fn);
- mod_delayed_work(system_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
+ mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
ret = devm_device_add_group(&hdev->dev, &oxp_cfg_attrs_group);
if (ret)
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 2/2] HID: appletb-kdb: Replace system_wq with system_dfl_wq
From: Marco Crivellari @ 2026-07-07 14:53 UTC (permalink / raw)
To: linux-kernel, linux-input
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20260707145327.313292-1-marco.crivellari@suse.com>
Currently the code enqueue work items using mod_delayed_work(), using
system_wq, the old per-CPU Workqueue.
The function end up calling __queue_delayed_work(), which set a global
timer that could fire anywhere, enqueuing the work where the timer fired.
Unbound works could benefit from scheduler task placement, to optimize
performance and power consumption.
Since the workqueue work doesn't rely on per-cpu variables, there is no
obvious reason that justify the use of a per-cpu workqueue. So change the
workqueue with the new unbound version, system_dfl_wq.
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
drivers/hid/hid-appletb-kbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 462010a75899..5cc27066f602 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -175,7 +175,7 @@ static void appletb_inactivity_work(struct work_struct *work)
if (!kbd->has_dimmed) {
backlight_device_set_brightness(kbd->backlight_dev, 1);
kbd->has_dimmed = true;
- mod_delayed_work(system_wq, &kbd->inactivity_work,
+ mod_delayed_work(system_dfl_wq, &kbd->inactivity_work,
secs_to_jiffies(appletb_tb_idle_timeout));
} else if (!kbd->has_turned_off) {
backlight_device_set_brightness(kbd->backlight_dev, 0);
@@ -201,7 +201,7 @@ static void reset_inactivity_timer(struct appletb_kbd *kbd)
kbd->has_turned_off = false;
schedule_work(&kbd->restore_brightness_work);
}
- mod_delayed_work(system_wq, &kbd->inactivity_work,
+ mod_delayed_work(system_dfl_wq, &kbd->inactivity_work,
secs_to_jiffies(appletb_tb_dim_timeout));
}
}
@@ -423,7 +423,7 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
INIT_DELAYED_WORK(&kbd->inactivity_work, appletb_inactivity_work);
INIT_WORK(&kbd->restore_brightness_work,
appletb_restore_brightness_work);
- mod_delayed_work(system_wq, &kbd->inactivity_work,
+ mod_delayed_work(system_dfl_wq, &kbd->inactivity_work,
secs_to_jiffies(appletb_tb_dim_timeout));
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v5 0/6] gpiolib: fence off legacy interfaces
From: Arnd Bergmann @ 2026-07-07 14:55 UTC (permalink / raw)
To: Dmitry Torokhov, Arnd Bergmann
Cc: open list:GPIO SUBSYSTEM, John Paul Adrian Glaubitz,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Linus Walleij, Bartosz Golaszewski, Lee Jones,
Pavel Machek, linux-sh, linux-kernel, linux-input, linux-leds
In-Reply-To: <akxjQtCTcx2TOQMk@google.com>
On Tue, Jul 7, 2026, at 04:25, Dmitry Torokhov wrote:
> On Mon, Jun 29, 2026 at 03:03:23PM +0200, Arnd Bergmann wrote:
>>
>> I hope that all that remains now can just get merged through the
>> gpio tree. The gpio-keys patch needs a bit coordination with
>> another patch addressing the same issue that is already in
>> flight, so I expect that I'll rebase my series once more when
>> that is in a stable branch, but the state I have here should
>> just work as-is on top of v7.2-rc1.
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> # for input
Thanks
> Could this be done as an immutable branch?
Probably, yes. How about I send v6 now, with the two input
patches moved to the front of the series? Then all six of my
patches can go into the gpio tree, while Lee merges the bottom
two into the mfd tree as an immutable branch to resolve the
conflict with your rohm patches.
Arnd
^ permalink raw reply
* Re: [PATCH RFC 2/2] HID: appletb-kdb: Replace system_wq with system_dfl_wq
From: sashiko-bot @ 2026-07-07 15:04 UTC (permalink / raw)
To: Marco Crivellari; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707145327.313292-3-marco.crivellari@suse.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] Incomplete migration to the unbound workqueue: `schedule_work()` still targets the per-CPU workqueue.
Pre-existing issues:
- [High] Race condition during device probe causes a NULL pointer dereference if a HID event arrives before `hid_set_drvdata()` is called.
- [High] Sleeping in atomic context when calling `hid_hw_power` from the input event handler.
--
--- Patch [2]: [PATCH RFC 2/2] HID: appletb-kdb: Replace system_wq with system_dfl_wq ---
commit b55f729cf441f481f051cec559ad520a64ea4188
Author: Marco Crivellari <marco.crivellari@suse.com>
HID: appletb-kdb: Replace system_wq with system_dfl_wq
This commit changes the driver to use the unbound system_dfl_wq workqueue
instead of the per-CPU system_wq, to optimize task placement and power
consumption.
> diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
> index 462010a758993..5cc27066f6026 100644
> --- a/drivers/hid/hid-appletb-kbd.c
> +++ b/drivers/hid/hid-appletb-kbd.c
[ ... ]
> @@ -201,7 +201,7 @@ static void reset_inactivity_timer(struct appletb_kbd *kbd)
> kbd->has_turned_off = false;
> schedule_work(&kbd->restore_brightness_work);
> }
[Severity: Low]
Since the goal is to move entirely to the unbound system_dfl_wq, did we miss
updating this schedule_work() call?
The schedule_work() function implicitly targets the per-CPU system_wq, so
restore_brightness_work won't benefit from the scheduler optimizations
intended by this patch.
Could we use queue_work() with system_dfl_wq here instead?
> - mod_delayed_work(system_wq, &kbd->inactivity_work,
> + mod_delayed_work(system_dfl_wq, &kbd->inactivity_work,
> secs_to_jiffies(appletb_tb_dim_timeout));
> }
> }
[ ... ]
> @@ -423,7 +423,7 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
> INIT_DELAYED_WORK(&kbd->inactivity_work, appletb_inactivity_work);
> INIT_WORK(&kbd->restore_brightness_work,
> appletb_restore_brightness_work);
> - mod_delayed_work(system_wq, &kbd->inactivity_work,
> + mod_delayed_work(system_dfl_wq, &kbd->inactivity_work,
> secs_to_jiffies(appletb_tb_dim_timeout));
> }
[Severity: High]
This isn't a bug introduced by this patch, but looking at the surrounding
appletb_kbd_probe() setup, is there a race condition between hid_hw_open()
and driver data initialization?
Earlier in appletb_kbd_probe(), hid_hw_open() is called before
hid_set_drvdata() executes. If a hardware event arrives immediately after
hid_hw_open() starts polling, the hid core calls the event handler:
appletb_kbd_hid_event()
struct appletb_kbd *kbd = hid_get_drvdata(hdev);
Because hid_set_drvdata() hasn't been called yet, hid_get_drvdata() returns
NULL. When reset_inactivity_timer() later dereferences kbd->backlight_dev,
could this cause a kernel panic?
[Severity: High]
This is another pre-existing issue, but does appletb_kbd_inp_event() sleep
in an atomic context?
When the input subsystem invokes the .event callback, it holds the device's
event_lock spinlock with local interrupts disabled. If the fn key is toggled,
the callback path goes:
appletb_kbd_inp_event()
appletb_kbd_set_mode()
hid_hw_power(hdev, PM_HINT_FULLON);
For usb devices, hid_hw_power() resolves to usbhid_power(), which calls
usb_autopm_get_interface(). This uses pm_runtime_get_sync(), which acquires a
mutex and can sleep.
Could this trigger a scheduling while atomic panic when the user presses
the fn key?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707145327.313292-1-marco.crivellari@suse.com?part=2
^ permalink raw reply
* Re: [PATCH RFC 1/2] HID: hid-oxp: Replace system_wq with system_dfl_wq
From: sashiko-bot @ 2026-07-07 15:05 UTC (permalink / raw)
To: Marco Crivellari; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260707145327.313292-2-marco.crivellari@suse.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The driver uses a single global statically allocated LED class device, causing list corruption and state clobbering if multiple matching devices are connected.
- [High] Inadequate teardown of delayed works causes Use-After-Free vulnerabilities during device removal and probe failures.
- [High] Handling a GEN2 status event unconditionally dereferences `drvdata.led_mc`, which is NULL on hybrid MCU devices.
- [High] The driver blindly accesses HID event data without validating the `size` parameter, leading to out-of-bounds reads.
--
--- Patch [1]: [PATCH RFC 1/2] HID: hid-oxp: Replace system_wq with system_dfl_wq ---
commit 17346571c8ccd09d42318cd808c4c65570f51a89
Author: Marco Crivellari <marco.crivellari@suse.com>
HID: hid-oxp: Replace system_wq with system_dfl_wq
The function end up calling __queue_delayed_work(), which set a global
timer that could fire anywhere, enqueuing the work where the timer fired.
Unbound works could benefit from scheduler task placement, to optimize
performance and power consumption.
Since the workqueue work doesn't rely on per-cpu variables, there is no
obvious reason that justify the use of a per-cpu workqueue. So change the
workqueue with the new unbound version, system_dfl_wq.
> diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
> index 20a54f337220d..d2ded6b08ce9e 100644
> --- a/drivers/hid/hid-oxp.c
> +++ b/drivers/hid/hid-oxp.c
> @@ -398,7 +398,7 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
> * Re-apply our settings after this has been received.
> */
> if (data[3] == OXP_EFFECT_MONO_TRUE) {
> - mod_delayed_work(system_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
> + mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
[Severity: High]
This isn't a bug introduced by this patch, but does the driver validate the
size parameter before accessing data[0] or data[3] in this function?
It appears that if a device sends a smaller report than expected, we could hit
an out-of-bounds read when checking data[3] or when dumping OXP_PACKET_SIZE
bytes in oxp_hid_raw_event().
[Severity: High]
This is a pre-existing issue, but is it possible for drvdata.led_mc to be NULL
here?
If oxp_cfg_probe() jumps to skip_rgb for hybrid MCU devices, drvdata.led_mc
is left uninitialized (NULL). If the device later sends a GEN2 status event,
this function eventually does:
led_mc->led_cdev.brightness = rgb_rep->brightness / 4 * ...
Could this cause a NULL pointer dereference on hybrid devices?
> return 0;
> }
>
[ ... ]
> @@ -1349,7 +1349,7 @@ static void oxp_rgb_brightness_set(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> led_cdev->brightness = brightness;
> - mod_delayed_work(system_wq, &drvdata.oxp_rgb_queue, msecs_to_jiffies(50));
> + mod_delayed_work(system_dfl_wq, &drvdata.oxp_rgb_queue, msecs_to_jiffies(50));
[Severity: High]
This is a pre-existing issue, but can this delayed work cause a use-after-free
during device removal?
Because oxp_hid_remove() uses cancel_delayed_work() instead of the synchronous
cancel_delayed_work_sync(), the work might still be running. Also, when devres
unregisters the LED classdev after remove() finishes, the LED core calls this
function, which unconditionally re-arms the workqueue.
Could the workqueue execute after devres has freed memory structures, leading
to a use-after-free?
> }
>
[ ... ]
> @@ -1502,7 +1502,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
> drvdata.rumble_intensity = 5;
>
> INIT_DELAYED_WORK(&drvdata.oxp_mcu_init, oxp_mcu_init_fn);
> - mod_delayed_work(system_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
> + mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
>
> ret = devm_device_add_group(&hdev->dev, &oxp_cfg_attrs_group);
> if (ret)
[Severity: High]
This isn't a bug introduced by this patch, but if devm_device_add_group()
fails here, the probe function returns the error directly.
Does this leave the oxp_mcu_init delayed work scheduled, potentially causing a
use-after-free if the work runs after the device and its devres allocations
have been freed?
[Severity: Critical]
This is a pre-existing issue, but does oxp_cfg_probe() register a single
globally statically allocated led class device?
Earlier in oxp_cfg_probe():
ret = devm_led_classdev_multicolor_register(&hdev->dev, &oxp_cdev_rgb);
Since oxp_cdev_rgb is statically allocated, if multiple matching HID devices
are connected, we will call devm_led_classdev_multicolor_register() multiple
times with the same pointer. Could this corrupt the LED subsystem's internal
linked list and cause a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707145327.313292-1-marco.crivellari@suse.com?part=1
^ permalink raw reply
* Re: [PATCH v2] HID: hid-lenovo-go: cancel cfg_setup work in hid_go_cfg_remove()
From: Derek J. Clark @ 2026-07-07 15:19 UTC (permalink / raw)
To: Günther Noack
Cc: Manish Khadka, dmitry.torokhov, linux-input, sashiko-reviews,
Jiri Kosina, Benjamin Tissoires, Lee Jones, Mark Pearson
In-Reply-To: <akzBhKTOVpaTb4nT@google.com>
On July 7, 2026 2:06:12 AM PDT, "Günther Noack" <gnoack@google.com> wrote:
>Hello Derek and Mark!
>
>May I please draw your attention to this issue in the hid-lenovo drivers,
>which was pointed out by Sashiko:
>
>
>On Fri, May 15, 2026 at 06:23:46PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
>> - [Critical] Hardware state corruption and crashes due to use of a static global struct for device data.
>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>
>> > diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
>> > index d4d26c7833563..b9d8cde531365 100644
>> > --- a/drivers/hid/hid-lenovo-go.c
>> > +++ b/drivers/hid/hid-lenovo-go.c
>> > @@ -2408,6 +2408,15 @@ static int hid_go_cfg_probe(struct hid_device *hdev,
>> >
>> > static void hid_go_cfg_remove(struct hid_device *hdev)
>> > {
>> > + /*
>> > + * cfg_setup is scheduled from hid_go_cfg_probe() with a 2 ms delay
>> > + * and dereferences drvdata.hdev. Drain it here before tearing
>> > + * down so the workqueue cannot run after hid_destroy_device()'s
>> > + * put_device() has released the underlying hdev and dereference
>> > + * a stale drvdata.hdev pointer.
>> > + */
>> > + cancel_delayed_work_sync(&drvdata.go_cfg_setup);
>>
>> This isn't a bug introduced by this patch, but does the driver use a global
>> static variable (static struct hid_go_cfg drvdata) to store device state?
>>
>> A second device probe would unconditionally overwrite drvdata.hdev and
>> re-initialize both drvdata.cfg_mutex and drvdata.go_cfg_setup.
>>
>> Re-initializing an active mutex destroys its wait lists, and re-initializing
>> an active delayed work overwrites its linked list pointers, leading to
>> immediate memory corruption and kernel panics.
>>
>> Should the device state be dynamically allocated per-instance instead?
>
>
>This is the same issue which was also introduced in hid-oxp, which I
>pointed out in [1]. As the review-bot points out above, while the data in
>your static drvdata variable is maybe only necessary once (but that
>also seems questionable?), I think the mutex and workqueue shouldn't
>just get re-initialized in the middle of being used, when a second device is
>plugged in and the probe() hook runs again.
>
Hi Günther,
While this is technically correct, the likelihood of this happening is realistically zero. All of these drivers are used for handheld devices with built-in HID controllers. There is nothing to "plug-in" per say.
That being said, I do intend to return to these drivers and fix that issue. I was waiting to do that after the claw driver is merged as it is essentially dealing with the same issues and that will provide a blueprint for all the needed changes in the others.
Thanks,
Derek
>This pattern is used in both the drivers hid-lenovo-go.c, hid-lenovo-go-s.c,
>as well as in the newer hid-oxp discussed in [1].
>
>Shouldn't the drvdata just be allocated with one of the devm_*alloc() variants
>as in the other drivers?
>
>—Günther
>
>
>[1] https://lore.kernel.org/all/akfGTjId84EjV141@google.com/
^ permalink raw reply
* [PATCH v2 00/10] HID: steam: General cleanup and improvements
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
This is the first half of a patch series improving the hid-steam driver.
This series contains a few notable changes:
- As I've been maintaining it downstream in SteamOS as well as writing
general improvements, I'm marking myself as the maintainer of the driver.
- Renames some stuff, notably marking the original Steam Controller as
Steam Controller (2015) in preparation of adding support for the Steam
Controller (2026), which will be in the second half of this series once
it's ready for submission.
- Adds IMU support for the Steam Controller (2015).
- Some reliability improvements.
- Fixes a short read issue reported by syzkaller.
Hopefully the second half of the series will be ready soon, but I am
waiting on more testing by SteamOS users before I submit it.
This is a minor interation on v1 that fixes the short read check as well as
some miscellaneous issues found by sashiko.
Vicki Pfau (10):
HID: steam: Update documentation
HID: steam: Refactor and clean up report parsing
HID: steam: Rename some constants that got renamed upstream
HID: steam: Add support for sensor events on the Steam Controller
(2015)
HID: steam: Coalesce rumble packets
HID: steam: Fully unregister controller when hidraw is opened
HID: steam: Rearrange deinitialization sequence
HID: steam: Improve logging and other cleanup
HID: steam: Reject short reads
HID: steam: Retry send/recv reports if stale
MAINTAINERS | 6 +
drivers/hid/hid-steam.c | 669 +++++++++++++++++++++++++++-------------
2 files changed, 463 insertions(+), 212 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v2 01/10] HID: steam: Update documentation
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
Mark myself as the maintainer, as well as adding myself as an author.
It also makes some minor updates to comments, such as correcly calling the
left menu key view and retroactively renaming the original Steam Controller
as Steam Controller (2015), in preparation for support for the 2026 model.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
MAINTAINERS | 6 ++++++
drivers/hid/hid-steam.c | 13 +++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4ecd282f8f52..2ed29e970023 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11500,6 +11500,12 @@ F: drivers/hid/hid-sensor-*
F: drivers/iio/*/hid-*
F: include/linux/hid-sensor-*
+HID STEAM CONTROLLER
+M: Vicki Pfau <vi@endrift.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-steam.c
+
HID VRC-2 CAR CONTROLLER DRIVER
M: Marcus Folkesson <marcus.folkesson@gmail.com>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 197126d6e081..a854d6360a0e 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -48,6 +48,7 @@
MODULE_DESCRIPTION("HID driver for Valve Steam Controller");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
static bool lizard_mode = true;
@@ -1413,9 +1414,9 @@ static inline s16 steam_le16(u8 *data)
* 9.1 | BTN_DPAD_RIGHT | left-pad right
* 9.2 | BTN_DPAD_LEFT | left-pad left
* 9.3 | BTN_DPAD_DOWN | left-pad down
- * 9.4 | BTN_SELECT | menu left
+ * 9.4 | BTN_SELECT | view
* 9.5 | BTN_MODE | steam logo
- * 9.6 | BTN_START | menu right
+ * 9.6 | BTN_START | menu
* 9.7 | BTN_GRIPL | left back lever
* 10.0 | BTN_GRIPR | right back lever
* 10.1 | -- | left-pad clicked
@@ -1541,9 +1542,9 @@ static void steam_do_input_event(struct steam_device *steam,
* 9.1 | BTN_DPAD_RIGHT | left-pad right
* 9.2 | BTN_DPAD_LEFT | left-pad left
* 9.3 | BTN_DPAD_DOWN | left-pad down
- * 9.4 | BTN_SELECT | menu left
+ * 9.4 | BTN_SELECT | view
* 9.5 | BTN_MODE | steam logo
- * 9.6 | BTN_START | menu right
+ * 9.6 | BTN_START | menu
* 9.7 | BTN_GRIPL2 | left bottom grip button
* 10.0 | BTN_GRIPR2 | right bottom grip button
* 10.1 | BTN_THUMB | left pad pressed
@@ -1850,11 +1851,11 @@ MODULE_PARM_DESC(lizard_mode,
"Enable mouse and keyboard emulation (lizard mode) when the gamepad is not in use");
static const struct hid_device_id steam_controllers[] = {
- { /* Wired Steam Controller */
+ { /* Wired Steam Controller (2015) */
HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
USB_DEVICE_ID_STEAM_CONTROLLER)
},
- { /* Wireless Steam Controller */
+ { /* Wireless Steam Controller (2015) */
HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
.driver_data = STEAM_QUIRK_WIRELESS
--
2.54.0
^ permalink raw reply related
* [PATCH v2 03/10] HID: steam: Rename some constants that got renamed upstream
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-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 v2 02/10] HID: steam: Refactor and clean up report parsing
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
This switches from a parsing style where each button or axis is parsed
individually out of a report using !!(byte & BIT(x)) style. This commit
switches it to a mostly unified approach of defining a list of individual
mappings in an array and passing it to a function that handles all of the
extraction. Theoretically this is more lines, but in practice it results in
(subjectively) cleaner code. Some exceptions still need to be made for
things like handling the lizard mode toggle key, but in general there's a
lot less manual code.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 211 ++++++++++++++++++++++++----------------
1 file changed, 128 insertions(+), 83 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index a854d6360a0e..1b0367c98eac 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -43,6 +43,7 @@
#include <linux/rcupdate.h>
#include <linux/delay.h>
#include <linux/power_supply.h>
+#include <linux/unaligned.h>
#include "hid-ids.h"
MODULE_DESCRIPTION("HID driver for Valve Steam Controller");
@@ -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);
return x == -32768 ? -32767 : x;
}
+struct steam_button_mapping {
+ int code;
+ u8 byte;
+ u8 bit;
+};
+
+struct steam_axis_mapping {
+ int code;
+ s8 sign;
+ u8 byte;
+};
+
+static void steam_map_buttons(struct input_dev *input,
+ const struct steam_button_mapping *mappings, const u8 *data)
+{
+ const struct steam_button_mapping *mapping;
+
+ for (mapping = mappings; mapping->code; mapping++)
+ input_report_key(input, mapping->code,
+ data[mapping->byte] & BIT(mapping->bit));
+}
+
+static void steam_map_axes(struct input_dev *input,
+ const struct steam_axis_mapping *mappings, const u8 *data)
+{
+ const struct steam_axis_mapping *mapping;
+
+ for (mapping = mappings; mapping->sign; mapping++)
+ input_report_abs(input, mapping->code,
+ mapping->sign * steam_le16(&data[mapping->byte]));
+}
+
/*
* The size for this message payload is 60.
* The known values are:
@@ -1428,18 +1461,42 @@ static inline s16 steam_le16(u8 *data)
* 10.7 | -- | lpad_and_joy
*/
+static const struct steam_button_mapping steam_controller_button_mappings[] = {
+ { BTN_TR2, 8, 0 },
+ { BTN_TL2, 8, 1 },
+ { BTN_TR, 8, 2 },
+ { BTN_TL, 8, 3 },
+ { BTN_Y, 8, 4 },
+ { BTN_B, 8, 5 },
+ { BTN_X, 8, 6 },
+ { BTN_A, 8, 7 },
+ { BTN_SELECT, 9, 4 },
+ { BTN_MODE, 9, 5 },
+ { BTN_START, 9, 6 },
+ { BTN_GRIPL, 9, 7 },
+ { BTN_GRIPR, 10, 0 },
+ { BTN_THUMBR, 10, 2 },
+ { BTN_THUMBL, 10, 6 },
+ { BTN_THUMB2, 10, 4 },
+ { BTN_DPAD_UP, 9, 0 },
+ { BTN_DPAD_RIGHT, 9, 1 },
+ { BTN_DPAD_LEFT, 9, 2 },
+ { BTN_DPAD_DOWN, 9, 3 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_controller_axis_mappings[] = {
+ { ABS_RX, 1, 20 },
+ { ABS_RY, -1, 22 },
+ { /* sentinel */ },
+};
+
static void steam_do_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
- /* 24 bits of buttons */
- u8 b8, b9, b10;
s16 x, y;
bool lpad_touched, lpad_and_joy;
- b8 = data[8];
- b9 = data[9];
- b10 = data[10];
-
input_report_abs(input, ABS_HAT2Y, data[11]);
input_report_abs(input, ABS_HAT2X, data[12]);
@@ -1451,8 +1508,8 @@ static void steam_do_input_event(struct steam_device *steam,
* joystick values.
* (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
*/
- lpad_touched = b10 & BIT(3);
- lpad_and_joy = b10 & BIT(7);
+ lpad_touched = data[10] & BIT(3);
+ lpad_and_joy = data[10] & BIT(7);
x = steam_le16(data + 16);
y = -steam_le16(data + 18);
@@ -1468,31 +1525,10 @@ static void steam_do_input_event(struct steam_device *steam,
input_report_abs(input, ABS_HAT0X, 0);
input_report_abs(input, ABS_HAT0Y, 0);
}
+ input_report_key(input, BTN_THUMB, lpad_touched || lpad_and_joy);
- input_report_abs(input, ABS_RX, steam_le16(data + 20));
- input_report_abs(input, ABS_RY, -steam_le16(data + 22));
-
- input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
- input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
- input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
- input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
- input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
- input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
- input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
- input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
- input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
- input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
- input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
- input_event(input, EV_KEY, BTN_GRIPL, !!(b9 & BIT(7)));
- input_event(input, EV_KEY, BTN_GRIPR, !!(b10 & BIT(0)));
- input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
- input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
- input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
- input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
- input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
- input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
- input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
+ steam_map_buttons(input, steam_controller_button_mappings, data);
+ steam_map_axes(input, steam_controller_axis_mappings, data);
input_sync(input);
}
@@ -1595,23 +1631,67 @@ static void steam_do_input_event(struct steam_device *steam,
* 15.6 | -- | unknown
* 15.7 | -- | unknown
*/
+
+static const struct steam_button_mapping steam_deck_button_mappings[] = {
+ { BTN_TR2, 8, 0 },
+ { BTN_TL2, 8, 1 },
+ { BTN_TR, 8, 2 },
+ { BTN_TL, 8, 3 },
+ { BTN_Y, 8, 4 },
+ { BTN_B, 8, 5 },
+ { BTN_X, 8, 6 },
+ { BTN_A, 8, 7 },
+ { BTN_SELECT, 9, 4 },
+ { BTN_MODE, 9, 5 },
+ { BTN_START, 9, 6 },
+ { BTN_GRIPL2, 9, 7 },
+ { BTN_GRIPR2, 10, 0 },
+ { BTN_THUMBL, 10, 6 },
+ { BTN_THUMBR, 11, 2 },
+ { BTN_DPAD_UP, 9, 0 },
+ { BTN_DPAD_RIGHT, 9, 1 },
+ { BTN_DPAD_LEFT, 9, 2 },
+ { BTN_DPAD_DOWN, 9, 3 },
+ { BTN_THUMB, 10, 1 },
+ { BTN_THUMB2, 10, 2 },
+ { BTN_GRIPL, 13, 1 },
+ { BTN_GRIPR, 13, 2 },
+ { BTN_BASE, 14, 2 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_deck_axis_mappings[] = {
+ { ABS_X, 1, 48 },
+ { ABS_Y, -1, 50 },
+ { ABS_RX, 1, 52 },
+ { ABS_RY, -1, 54 },
+ { ABS_HAT2Y, 1, 44 },
+ { ABS_HAT2X, 1, 46 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_deck_imu_mappings[] = {
+ { ABS_X, 1, 24 },
+ { ABS_Z, -1, 26 },
+ { ABS_Y, 1, 28 },
+ { ABS_RX, 1, 30 },
+ { ABS_RZ, -1, 32 },
+ { ABS_RY, 1, 34 },
+ { /* sentinel */ },
+};
+
static void steam_do_deck_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
- u8 b8, b9, b10, b11, b13, b14;
+ bool start_pressed;
bool lpad_touched, rpad_touched;
- b8 = data[8];
- b9 = data[9];
- b10 = data[10];
- b11 = data[11];
- b13 = data[13];
- b14 = data[14];
+ start_pressed = data[9] & BIT(6);
- if (!(b9 & BIT(6)) && steam->did_mode_switch) {
+ if (!start_pressed && steam->did_mode_switch) {
steam->did_mode_switch = false;
cancel_delayed_work(&steam->mode_switch);
- } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) {
+ } else if (!steam->client_opened && start_pressed && !steam->did_mode_switch) {
steam->did_mode_switch = true;
schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
}
@@ -1619,8 +1699,8 @@ static void steam_do_deck_input_event(struct steam_device *steam,
if (!steam->gamepad_mode && lizard_mode)
return;
- lpad_touched = b10 & BIT(3);
- rpad_touched = b10 & BIT(4);
+ lpad_touched = data[10] & BIT(3);
+ rpad_touched = data[10] & BIT(4);
if (lpad_touched) {
input_report_abs(input, ABS_HAT0X, steam_le16(data + 16));
@@ -1638,38 +1718,8 @@ static void steam_do_deck_input_event(struct steam_device *steam,
input_report_abs(input, ABS_HAT1Y, 0);
}
- input_report_abs(input, ABS_X, steam_le16(data + 48));
- input_report_abs(input, ABS_Y, -steam_le16(data + 50));
- input_report_abs(input, ABS_RX, steam_le16(data + 52));
- input_report_abs(input, ABS_RY, -steam_le16(data + 54));
-
- input_report_abs(input, ABS_HAT2Y, steam_le16(data + 44));
- input_report_abs(input, ABS_HAT2X, steam_le16(data + 46));
-
- input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
- input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
- input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
- input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
- input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
- input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
- input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
- input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
- input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
- input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
- input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
- input_event(input, EV_KEY, BTN_GRIPL2, !!(b9 & BIT(7)));
- input_event(input, EV_KEY, BTN_GRIPR2, !!(b10 & BIT(0)));
- input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
- input_event(input, EV_KEY, BTN_THUMBR, !!(b11 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
- input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
- input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
- input_event(input, EV_KEY, BTN_THUMB, !!(b10 & BIT(1)));
- input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(2)));
- input_event(input, EV_KEY, BTN_GRIPL, !!(b13 & BIT(1)));
- input_event(input, EV_KEY, BTN_GRIPR, !!(b13 & BIT(2)));
- input_event(input, EV_KEY, BTN_BASE, !!(b14 & BIT(2)));
+ steam_map_buttons(input, steam_deck_button_mappings, data);
+ steam_map_axes(input, steam_deck_axis_mappings, data);
input_sync(input);
}
@@ -1690,12 +1740,7 @@ static void steam_do_deck_sensors_event(struct steam_device *steam,
return;
input_event(sensors, EV_MSC, MSC_TIMESTAMP, steam->sensor_timestamp_us);
- input_report_abs(sensors, ABS_X, steam_le16(data + 24));
- input_report_abs(sensors, ABS_Z, -steam_le16(data + 26));
- input_report_abs(sensors, ABS_Y, steam_le16(data + 28));
- input_report_abs(sensors, ABS_RX, steam_le16(data + 30));
- input_report_abs(sensors, ABS_RZ, -steam_le16(data + 32));
- input_report_abs(sensors, ABS_RY, steam_le16(data + 34));
+ steam_map_axes(sensors, steam_deck_imu_mappings, data);
input_sync(sensors);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-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
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