* [PATCH v12 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: Vicki Pfau @ 2026-07-15 3:34 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
Cc: Vicki Pfau, Silvan Jegen
In-Reply-To: <20260715033409.3599913-1-vi@endrift.com>
This adds rumble support for both the "HD Rumble" linear resonant actuator
type as used in the Joy-Cons and Pro Controller, as well as the eccentric
rotating mass type used in the GameCube controller. Note that since there's
currently no API for exposing full control of LRAs with evdev, it only
simulates a basic rumble for now.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/Kconfig | 8 +-
drivers/hid/hid-nintendo.c | 214 ++++++++++++++++++++++++++++++++++++-
2 files changed, 216 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c77c323ec9..851eed76c236 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -859,10 +859,10 @@ config NINTENDO_FF
depends on HID_NINTENDO
select INPUT_FF_MEMLESS
help
- Say Y here if you have a Nintendo Switch controller and want to enable
- force feedback support for it. This works for both joy-cons, the pro
- controller, and the NSO N64 controller. For the pro controller, both
- rumble motors can be controlled individually.
+ Say Y here if you have a Nintendo Switch or Switch 2 controller and want
+ to enable force feedback support for it. This works for Joy-Cons, the Pro
+ Controllers, and the NSO N64 and GameCube controller. For the Pro
+ Controller, both rumble motors can be controlled individually.
config HID_NTI
tristate "NTI keyboard adapters"
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 78c9ad3c1610..76eb4861ab23 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -37,6 +37,7 @@
#include <linux/unaligned.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/devm-helpers.h>
#include <linux/kernel.h>
#include <linux/hid.h>
#include <linux/idr.h>
@@ -2989,6 +2990,7 @@ enum switch2_init_step {
NS2_INIT_READ_USER_SECONDARY_CALIB,
NS2_INIT_SET_FEATURE_MASK,
NS2_INIT_ENABLE_FEATURES,
+ NS2_INIT_ENABLE_RUMBLE,
NS2_INIT_GRIP_BUTTONS,
NS2_INIT_REPORT_FORMAT,
NS2_INIT_INPUT,
@@ -3020,6 +3022,18 @@ struct switch2_stick_calibration {
struct switch2_axis_calibration y;
};
+struct switch2_hd_rumble {
+ uint16_t hi_freq : 10;
+ uint16_t hi_amp : 10;
+ uint16_t lo_freq : 10;
+ uint16_t lo_amp : 10;
+} __packed;
+
+struct switch2_erm_rumble {
+ uint16_t error;
+ uint16_t amplitude;
+};
+
struct switch2_controller {
struct hid_device *hdev;
struct switch2_cfg_intf *cfg;
@@ -3043,8 +3057,45 @@ struct switch2_controller {
uint32_t player_id;
struct led_classdev *leds;
+
+#if IS_ENABLED(CONFIG_NINTENDO_FF)
+ spinlock_t rumble_lock;
+ uint8_t rumble_seq;
+ union {
+ struct switch2_hd_rumble hd;
+ struct switch2_erm_rumble sd;
+ } rumble;
+ uint64_t last_rumble_work;
+ struct delayed_work rumble_work;
+ uint8_t *rumble_buffer;
+#endif
+};
+
+enum gc_rumble {
+ GC_RUMBLE_OFF = 0,
+ GC_RUMBLE_ON = 1,
+ GC_RUMBLE_STOP = 2,
};
+/*
+ * The highest rumble level for "HD Rumble" is strong enough to potentially damage the controller,
+ * and also leaves your hands feeling like melted jelly, so we set a semi-arbitrary scaling factor
+ * to artificially limit the maximum for safety and comfort. It is currently unknown if the Switch
+ * 2 itself does something similar, but it's quite likely.
+ *
+ * This value must be between 0 and 1024, otherwise the math below will overflow.
+ */
+#define RUMBLE_MAX 450u
+
+/*
+ * Semi-arbitrary values used to simulate the "rumble" sensation of an eccentric rotating
+ * mass type haptic motor on the Switch 2 controllers' linear resonant actuator type haptics.
+ *
+ * The units used are unknown, but the values must be between 0 and 1023.
+ */
+#define RUMBLE_HI_FREQ 0x187
+#define RUMBLE_LO_FREQ 0x112
+
static DEFINE_MUTEX(switch2_controllers_lock);
static LIST_HEAD(switch2_controllers);
@@ -3136,7 +3187,7 @@ static const uint8_t switch2_init_cmd_data[] = {
static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
static const uint8_t switch2_feature_mask[] = {
- NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU,
+ NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU | NS2_FEATURE_RUMBLE,
0x00, 0x00, 0x00
};
@@ -3209,6 +3260,128 @@ static void switch2_kref_put(struct kref *refcount)
kfree(ns2);
}
+#if IS_ENABLED(CONFIG_NINTENDO_FF)
+static void switch2_encode_rumble(struct switch2_hd_rumble *rumble, uint8_t buffer[5])
+{
+ buffer[0] = rumble->hi_freq;
+ buffer[1] = (rumble->hi_freq >> 8) | (rumble->hi_amp << 2);
+ buffer[2] = (rumble->hi_amp >> 6) | (rumble->lo_freq << 4);
+ buffer[3] = (rumble->lo_freq >> 4) | (rumble->lo_amp << 6);
+ buffer[4] = rumble->lo_amp >> 2;
+}
+
+static int switch2_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
+{
+ struct switch2_controller *ns2 = input_get_drvdata(dev);
+ unsigned long flags;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ if (!ns2)
+ return -ENODEV;
+
+ spin_lock_irqsave(&ns2->rumble_lock, flags);
+ if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
+ ns2->rumble.sd.amplitude = max(effect->u.rumble.strong_magnitude,
+ (uint16_t) (effect->u.rumble.weak_magnitude >> 1));
+ } else {
+ ns2->rumble.hd.hi_freq = RUMBLE_HI_FREQ;
+ ns2->rumble.hd.lo_freq = RUMBLE_LO_FREQ;
+ ns2->rumble.hd.hi_amp = effect->u.rumble.weak_magnitude * RUMBLE_MAX >> 16;
+ ns2->rumble.hd.lo_amp = effect->u.rumble.strong_magnitude * RUMBLE_MAX >> 16;
+ }
+ spin_unlock_irqrestore(&ns2->rumble_lock, flags);
+
+ schedule_delayed_work(&ns2->rumble_work, 0);
+
+ return 0;
+}
+
+static void switch2_rumble_work(struct work_struct *work)
+{
+ struct switch2_controller *ns2 = container_of(to_delayed_work(work),
+ struct switch2_controller, rumble_work);
+ unsigned long flags;
+ bool active;
+ int ret = 0;
+
+ spin_lock_irqsave(&ns2->rumble_lock, flags);
+ ns2->rumble_buffer[0x1] = 0x50 | ns2->rumble_seq;
+ if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
+ ns2->rumble_buffer[0] = 3;
+ if (ns2->rumble.sd.amplitude == 0) {
+ ns2->rumble_buffer[2] = GC_RUMBLE_STOP;
+ ns2->rumble.sd.error = 0;
+ active = false;
+ } else {
+ if (ns2->rumble.sd.error < ns2->rumble.sd.amplitude) {
+ ns2->rumble_buffer[2] = GC_RUMBLE_ON;
+ ns2->rumble.sd.error += U16_MAX - ns2->rumble.sd.amplitude;
+ } else {
+ ns2->rumble_buffer[2] = GC_RUMBLE_OFF;
+ ns2->rumble.sd.error -= ns2->rumble.sd.amplitude;
+ }
+ active = true;
+ }
+ } else {
+ ns2->rumble_buffer[0] = 1;
+ switch2_encode_rumble(&ns2->rumble.hd, &ns2->rumble_buffer[0x2]);
+ active = ns2->rumble.hd.hi_amp || ns2->rumble.hd.lo_amp;
+ if (ns2->ctlr_type == NS2_CTLR_TYPE_PRO) {
+ /*
+ * The Pro Controller contains separate LRAs on each
+ * side that can be controlled individually.
+ */
+ ns2->rumble_buffer[0] = 2;
+ ns2->rumble_buffer[0x11] = 0x50 | ns2->rumble_seq;
+ switch2_encode_rumble(&ns2->rumble.hd, &ns2->rumble_buffer[0x12]);
+ }
+ }
+ ns2->rumble_seq = (ns2->rumble_seq + 1) & 0xF;
+ spin_unlock_irqrestore(&ns2->rumble_lock, flags);
+
+ if (active) {
+ unsigned long interval = msecs_to_jiffies(4);
+ uint64_t current_jiffies = get_jiffies_64();
+
+ if (!ns2->last_rumble_work)
+ ns2->last_rumble_work = current_jiffies;
+ else
+ ns2->last_rumble_work += interval;
+
+ /* Reschedule a little early to make sure the buffer never underruns */
+ interval -= msecs_to_jiffies(2);
+ if (ns2->last_rumble_work + interval >= current_jiffies)
+ schedule_delayed_work(&ns2->rumble_work,
+ ns2->last_rumble_work + interval - current_jiffies);
+ else
+ schedule_delayed_work(&ns2->rumble_work, 0);
+ } else {
+ ns2->last_rumble_work = 0;
+ }
+
+ mutex_lock(&ns2->lock);
+ if (!ns2->hdev) {
+ cancel_delayed_work(&ns2->rumble_work);
+ } else {
+ ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
+ /*
+ * Don't log on ENODEV, ESHUTDOWN, or EPROTO, which can happen
+ * mid-hotplug. Also cancel any further work on ENODEV or
+ * ESHUTDOWN as they're clear indications that the endpoint
+ * is dead.
+ */
+ if (ret == -ENODEV || ret == -ESHUTDOWN)
+ cancel_delayed_work(&ns2->rumble_work);
+ else if (ret < 0 && ret != -EPROTO)
+ hid_warn_ratelimited(ns2->hdev,
+ "Failed to send output report ret=%d\n", ret);
+ }
+ mutex_unlock(&ns2->lock);
+}
+#endif
+
static int switch2_set_leds(struct switch2_controller *ns2)
{
int i;
@@ -3332,6 +3505,26 @@ static int switch2_init_input(struct switch2_controller *ns2)
return -EINVAL;
}
+#if IS_ENABLED(CONFIG_NINTENDO_FF)
+ ns2->rumble_buffer = devm_kzalloc(&input->dev, 64, GFP_KERNEL);
+ if (!ns2->rumble_buffer) {
+ input_free_device(input);
+ return -ENOMEM;
+ }
+ ret = devm_delayed_work_autocancel(&input->dev, &ns2->rumble_work, switch2_rumble_work);
+ if (ret < 0) {
+ input_free_device(input);
+ return ret;
+ }
+
+ input_set_capability(input, EV_FF, FF_RUMBLE);
+ ret = input_ff_create_memless(input, NULL, switch2_play_effect);
+ if (ret) {
+ input_free_device(input);
+ return ret;
+ }
+#endif
+
hid_info(ns2->hdev, "Firmware version %u.%u.%u (type %i)\n", ns2->version.major,
ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type);
if (ns2->version.dsp_type >= 0)
@@ -3764,7 +3957,16 @@ int switch2_init_controller(struct switch2_controller *ns2)
return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_SET_MASK,
switch2_feature_mask, sizeof(switch2_feature_mask), ns2->cfg);
case NS2_INIT_ENABLE_FEATURES:
- return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG);
+ return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS |
+ NS2_FEATURE_ANALOG | NS2_FEATURE_RUMBLE);
+ case NS2_INIT_ENABLE_RUMBLE:
+ /*
+ * It is unclear what this packet is supposed to be for, but it
+ * appears to be needed for rumble to work reliably. The reply
+ * data indicates it might be a query of some sort, but we
+ * ignore the reply so long as it doesn't return an error.
+ */
+ return ns2->cfg->send_command(0x11, 1, NULL, 0, ns2->cfg);
case NS2_INIT_GRIP_BUTTONS:
if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) {
switch2_init_step_done(ns2, ns2->init_step);
@@ -3877,6 +4079,10 @@ int switch2_receive_command(struct switch2_controller *ns2,
switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
}
break;
+ case 0x11:
+ if (header->subcommand == 1)
+ switch2_init_step_done(ns2, NS2_INIT_ENABLE_RUMBLE);
+ break;
default:
break;
}
@@ -3996,6 +4202,10 @@ static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id
else
ns2->player_id = ret;
+#if IS_ENABLED(CONFIG_NINTENDO_FF)
+ spin_lock_init(&ns2->rumble_lock);
+#endif
+
ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
if (ret) {
hid_err(hdev, "hw_start failed %d\n", ret);
--
2.54.0
^ permalink raw reply related
* [PATCH v12 0/3] HID: nintendo: Add preliminary Switch 2 controller
From: Vicki Pfau @ 2026-07-15 3:34 UTC (permalink / raw)
To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
Cc: Vicki Pfau, Silvan Jegen
This series adds preliminary support for Switch 2 controllers using the
same split-driver model as previous versions. This is a minor iteration on
v11, fixing some nitpicks. It should be fully ready now, pending review
from Dmitry and a HID tree maintainer.
Vicki Pfau (3):
HID: nintendo: Add preliminary Switch 2 controller driver
HID: nintendo: Add rumble support for Switch 2 controllers
HID: nintendo: Add unified report format support
MAINTAINERS | 1 +
drivers/hid/Kconfig | 19 +-
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-nintendo.c | 1682 ++++++++++++++++-
drivers/hid/hid-nintendo.h | 72 +
drivers/input/joystick/Kconfig | 11 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/nintendo-switch2-usb.c | 475 +++++
8 files changed, 2224 insertions(+), 41 deletions(-)
create mode 100644 drivers/hid/hid-nintendo.h
create mode 100644 drivers/input/joystick/nintendo-switch2-usb.c
--
2.54.0
^ permalink raw reply
* Re: [PATCH] Input: wacom_w8001 - replace strlcat() with a strscpy() helper
From: sashiko-bot @ 2026-07-15 1:32 UTC (permalink / raw)
To: Ian Bridges; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <albg4Rv7QxvLJD05@dev>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Calling `input_free_device()` on a successfully registered device after `input_unregister_device()` in the error path leads to a Use-After-Free / Double Free vulnerability.
--
--- Patch [1]: [PATCH] Input: wacom_w8001 - replace strlcat() with a strscpy() helper ---
commit e4f21712f81562322908d563a501b6a8bead0360
Author: Ian Bridges <icb@fastmail.org>
Input: wacom_w8001 - replace strlcat() with a strscpy() helper
This commit replaces five uses of strlcat() with a custom strscpy()-based
helper, w8001_append_suffix(). This prepares for the eventual removal of
the strlcat() API.
Link: https://github.com/KSPP/linux/issues/370 [1]
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 45930d7318733..d8d1cdc3f09e3 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
[ ... ]
> @@ -534,7 +541,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
> input_abs_set_res(dev, ABS_MT_POSITION_X, touch.panel_res);
> input_abs_set_res(dev, ABS_MT_POSITION_Y, touch.panel_res);
>
> - strlcat(basename, " 2FG", basename_sz);
> + w8001_append_suffix(basename, " 2FG", basename_sz);
> if (w8001->max_pen_x && w8001->max_pen_y)
> w8001->id = 0xE3;
> else
> @@ -542,7 +549,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
> break;
> }
>
> - strlcat(basename, " Touchscreen", basename_sz);
> + w8001_append_suffix(basename, " Touchscreen", basename_sz);
>
> return 0;
> }
[Severity: High]
This isn't a bug introduced by this patch, but calling input_free_device()
on a successfully registered device after input_unregister_device() in the
error path of w8001_connect() leads to a double free.
If w8001_connect() successfully registers the pen device but subsequently
fails to register the touch device, execution jumps to the fail4 label:
drivers/input/touchscreen/wacom_w8001.c:w8001_connect() {
...
if (!err_touch) {
...
err = input_register_device(w8001->touch_dev);
if (err)
goto fail4;
...
fail4:
if (w8001->pen_dev)
input_unregister_device(w8001->pen_dev);
fail3:
serio_close(serio);
fail2:
serio_set_drvdata(serio, NULL);
fail1:
input_free_device(input_dev_pen);
input_free_device(input_dev_touch);
...
}
The fail4 block calls input_unregister_device(w8001->pen_dev), which drops
the device reference. Execution then falls through to the fail1 label, where
input_free_device(input_dev_pen) is unconditionally called. Because
input_dev_pen and w8001->pen_dev point to the same device, this results in a
second free.
Could this error path be updated to avoid the double free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/albg4Rv7QxvLJD05@dev?part=1
^ permalink raw reply
* [PATCH] Input: wacom_w8001 - replace strlcat() with a strscpy() helper
From: Ian Bridges @ 2026-07-15 1:22 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, linux-hardening
In preparation for removing the strlcat() API[1], replace its five
uses with a small append helper built on strnlen() and strscpy().
The five calls append device name fragments to a basename buffer
that grows in place across the setup functions. The helper takes
the same arguments as strlcat() and writes the same bytes, including
when a fragment is truncated.
Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
The produced device names are unchanged.
A seq_buf conversion was considered instead of the helper and set
aside for two reasons. seq_buf_puts() drops a whole fragment when it
does not fit, while strlcat() writes the partial fragment, so the
produced bytes would change at the truncation boundary. The setup
functions would also change signature to take a struct seq_buf
pointer. A v2 can adopt seq_buf if that tradeoff is preferred.
The patch was tested as follows.
- W=1 build of drivers/input/touchscreen/, zero warnings.
- A userspace differential harness compiled the old and the new
functions side by side. The name buffers were byte identical over
their full length, and the return values and id assignments matched
in every case.
drivers/input/touchscreen/wacom_w8001.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 45930d731873..d8d1cdc3f09e 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -417,6 +417,13 @@ static int w8001_detect(struct w8001 *w8001)
return 0;
}
+static void w8001_append_suffix(char *dest, const char *suffix, size_t dest_sz)
+{
+ size_t used = strnlen(dest, dest_sz);
+
+ strscpy(dest + used, suffix, dest_sz - used);
+}
+
static int w8001_setup_pen(struct w8001 *w8001, char *basename,
size_t basename_sz)
{
@@ -453,7 +460,7 @@ static int w8001_setup_pen(struct w8001 *w8001, char *basename,
}
w8001->id = 0x90;
- strlcat(basename, " Penabled", basename_sz);
+ w8001_append_suffix(basename, " Penabled", basename_sz);
return 0;
}
@@ -503,14 +510,14 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
case 2:
w8001->pktlen = W8001_PKTLEN_TOUCH93;
w8001->id = 0x93;
- strlcat(basename, " 1FG", basename_sz);
+ w8001_append_suffix(basename, " 1FG", basename_sz);
break;
case 1:
case 3:
case 4:
w8001->pktlen = W8001_PKTLEN_TOUCH9A;
- strlcat(basename, " 1FG", basename_sz);
+ w8001_append_suffix(basename, " 1FG", basename_sz);
w8001->id = 0x9a;
break;
@@ -534,7 +541,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
input_abs_set_res(dev, ABS_MT_POSITION_X, touch.panel_res);
input_abs_set_res(dev, ABS_MT_POSITION_Y, touch.panel_res);
- strlcat(basename, " 2FG", basename_sz);
+ w8001_append_suffix(basename, " 2FG", basename_sz);
if (w8001->max_pen_x && w8001->max_pen_y)
w8001->id = 0xE3;
else
@@ -542,7 +549,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
break;
}
- strlcat(basename, " Touchscreen", basename_sz);
+ w8001_append_suffix(basename, " Touchscreen", basename_sz);
return 0;
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] HID: add driver for Gigabyte Aero vendor-specific brightness keys
From: sashiko-bot @ 2026-07-15 1:08 UTC (permalink / raw)
To: Uddhav Swami; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260715004457.100650-1-uddhavswami@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dangling pointer in `drvdata->input` leads to Use-After-Free if `hidinput_connect()` fails during device probe.
--
--- Patch [1]: [PATCH] HID: add driver for Gigabyte Aero vendor-specific brightness keys ---
commit 1b9632575a5f9cf3b6d4d94e30945e582cc34a94
Author: Uddhav Swami <uddhavswami@gmail.com>
HID: add driver for Gigabyte Aero vendor-specific brightness keys
This commit adds a minimal HID driver that intercepts vendor-defined HID
reports for the Gigabyte Aero 15 XB keyboard. It maps values 0x7D and 0x7E
to KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP respectively.
> diff --git a/drivers/hid/hid-gigabyte.c b/drivers/hid/hid-gigabyte.c
> new file mode 100644
> index 0000000000000..1c242734aed3f
> --- /dev/null
> +++ b/drivers/hid/hid-gigabyte.c
> @@ -0,0 +1,127 @@
[ ... ]
> +static int gigabyte_aero_input_configured(struct hid_device *hdev,
> + struct hid_input *hi)
> +{
> + struct gigabyte_drvdata *drvdata = hid_get_drvdata(hdev);
> + struct input_dev *input = hi->input;
> +
> + input_set_capability(input, EV_KEY, KEY_BRIGHTNESSUP);
> + input_set_capability(input, EV_KEY, KEY_BRIGHTNESSDOWN);
> +
> + drvdata->input = input;
[Severity: High]
Can drvdata->input become a dangling pointer here if input registration fails?
If hidinput_connect() fails later during the probe sequence (for example, if
input_register_device() fails), hidinput_disconnect() will be called and it
frees the input_dev.
However, because this driver implements a .raw_event callback, hid_connect()
will still return success. This allows hid_hw_start() to return 0 and the
probe to complete successfully.
If that happens, drvdata->input is left pointing to the freed input_dev,
which will trigger a use-after-free in gigabyte_aero_raw_event() when a
brightness key is pressed:
case GIGABYTE_AERO_BRIGHTNESS_DOWN:
input_report_key(drvdata->input, KEY_BRIGHTNESSDOWN, 1);
Could we avoid this by checking if (hdev->claimed & HID_CLAIMED_INPUT) is
set after hid_hw_start() completes in gigabyte_aero_probe()?
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260715004457.100650-1-uddhavswami@gmail.com?part=1
^ permalink raw reply
* [PATCH] HID: add driver for Gigabyte Aero vendor-specific brightness keys
From: Uddhav Swami @ 2026-07-15 0:44 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, Uddhav Swami
The Gigabyte Aero 15 XB keyboard (Chu Yuen Enterprise Co., Ltd,
USB ID 1044:7a3f) sends brightness up/down keypresses as vendor-defined
HID reports (Usage Page 0xFF02, Report ID 4) rather than standard HID
Consumer Control usages, causing KEY_BRIGHTNESSUP and KEY_BRIGHTNESSDOWN
to never reach the input subsystem.
Add a minimal HID driver that intercepts Report ID 4 and maps values
0x7D and 0x7E to KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP respectively.
Tested on: Gigabyte Aero 15 XB (USB ID 1044:7a3f)
Signed-off-by: Uddhav Swami <uddhavswami@gmail.com>
---
drivers/hid/Kconfig | 10 +++
drivers/hid/Makefile | 1 +
drivers/hid/hid-gigabyte.c | 127 +++++++++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 5 +-
4 files changed, 141 insertions(+), 2 deletions(-)
create mode 100644 drivers/hid/hid-gigabyte.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 03f36899e458..fa74e67e35c8 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -406,6 +406,16 @@ config HID_GFRM
help
Support for Google Fiber TV Box remote controls
+config HID_GIGABYTE_AERO
+ tristate "Gigabyte Aero laptop vendor-specific keys"
+ depends on USB_HID
+ help
+ Support for vendor-specific keyboard keys on Gigabyte Aero
+ laptops
+
+ Currently the following device is known to be supported:
+ - Gigabyte Aero 15 XB
+
config HID_GLORIOUS
tristate "Glorious PC Gaming Race mice"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 23e6e3dd0c56..ef3a14b04126 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_FT260) += hid-ft260.o
obj-$(CONFIG_HID_GEMBIRD) += hid-gembird.o
obj-$(CONFIG_HID_GFRM) += hid-gfrm.o
+obj-$(CONFIG_HID_GIGABYTE_AERO) += hid-gigabyte.o
obj-$(CONFIG_HID_GLORIOUS) += hid-glorious.o
obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
obj-$(CONFIG_HID_GOODIX_SPI) += hid-goodix-spi.o
diff --git a/drivers/hid/hid-gigabyte.c b/drivers/hid/hid-gigabyte.c
new file mode 100644
index 000000000000..1c242734aed3
--- /dev/null
+++ b/drivers/hid/hid-gigabyte.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for Gigabyte Aero laptop vendor-specific brightness keys.
+ *
+ * The keyboard sends brightness up/down presses as a vendor-defined usage page
+ * report instead of standard HID Consumer Control usages.
+ *
+ * This driver intercepts them and emits the correct KEY_BRIGHTNESSUP /
+ * KEY_BRIGHTNESSDOWN events.
+ *
+ * Currently supported devices are:
+ * Gigabyte Aero 15 XB
+ *
+ * Copyright (c) 2026 Uddhav Swami <uddhavswami@gmail.com>
+ *
+ * This module based on hid-asus by
+ * Copyright (c) 2016 Yusuke Fujimaki <usk.fujimaki@gmail.com>
+ * Copyright (c) 2016 Brendan McGrath <redmcg@redmandi.dyndns.org>
+ * Copyright (c) 2016 Victor Vlasenko <victor.vlasenko@sysgears.com>
+ * Copyright (c) 2016 Frederik Wenigwieser <frederik.wenigwieser@gmail.com>
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/input.h>
+
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Uddhav Swami <uddhavswami@gmail.com>");
+MODULE_DESCRIPTION("HID driver for Gigabyte Aero");
+
+#define GIGABYTE_AERO_REPORT_ID 0x04
+#define GIGABYTE_AERO_BRIGHTNESS_DOWN 0x7D
+#define GIGABYTE_AERO_BRIGHTNESS_UP 0x7E
+
+struct gigabyte_drvdata {
+ struct input_dev *input;
+};
+
+static int gigabyte_aero_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct gigabyte_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata->input)
+ return 0;
+
+ if (report->id != GIGABYTE_AERO_REPORT_ID || size < 4)
+ return 0;
+
+ switch (data[3]) {
+ case GIGABYTE_AERO_BRIGHTNESS_DOWN:
+ input_report_key(drvdata->input, KEY_BRIGHTNESSDOWN, 1);
+ input_sync(drvdata->input);
+ input_report_key(drvdata->input, KEY_BRIGHTNESSDOWN, 0);
+ input_sync(drvdata->input);
+ return 1;
+ case GIGABYTE_AERO_BRIGHTNESS_UP:
+ input_report_key(drvdata->input, KEY_BRIGHTNESSUP, 1);
+ input_sync(drvdata->input);
+ input_report_key(drvdata->input, KEY_BRIGHTNESSUP, 0);
+ input_sync(drvdata->input);
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+static int gigabyte_aero_input_configured(struct hid_device *hdev,
+ struct hid_input *hi)
+{
+ struct gigabyte_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct input_dev *input = hi->input;
+
+ input_set_capability(input, EV_KEY, KEY_BRIGHTNESSUP);
+ input_set_capability(input, EV_KEY, KEY_BRIGHTNESSDOWN);
+
+ drvdata->input = input;
+
+ return 0;
+}
+
+static int gigabyte_aero_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct gigabyte_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ hid_set_drvdata(hdev, drvdata);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "gigabyte_aero: parse failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev, "gigabyte_aero: hw start failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct hid_device_id gigabyte_aero_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_CHU_YUEN,
+ USB_DEVICE_ID_CHU_YUEN_AERO_KBD) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, gigabyte_aero_devices);
+
+static struct hid_driver gigabyte_aero_driver = {
+ .name = "hid_gigabyte_aero",
+ .id_table = gigabyte_aero_devices,
+ .probe = gigabyte_aero_probe,
+ .raw_event = gigabyte_aero_raw_event,
+ .input_configured = gigabyte_aero_input_configured,
+};
+module_hid_driver(gigabyte_aero_driver);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b70f719b3b07..e33cdf33929b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -330,6 +330,9 @@
#define USB_VENDOR_ID_CHUNGHWAT 0x2247
#define USB_DEVICE_ID_CHUNGHWAT_MULTITOUCH 0x0001
+#define USB_VENDOR_ID_CHU_YUEN 0x1044
+#define USB_DEVICE_ID_CHU_YUEN_AERO_KBD 0x7a3f
+
#define USB_VENDOR_ID_CIDC 0x1677
#define I2C_VENDOR_ID_CIRQUE 0x0488
@@ -1319,7 +1322,6 @@
#define USB_DEVICE_ID_SMK_NSG_MR5U_REMOTE 0x0368
#define USB_DEVICE_ID_SMK_NSG_MR7U_REMOTE 0x0369
-
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
@@ -1610,7 +1612,6 @@
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D65 0x4d65
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22
-
#define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */
#define USB_DEVICE_ID_RI_KA_WEBMAIL 0x1320 /* Webmail Notifier */
--
2.55.0
^ permalink raw reply related
* Re: [PATCH 0/3] HID: wiimote: new LED behavior on connect, and scoped guards
From: Rafael Passos @ 2026-07-14 20:31 UTC (permalink / raw)
To: David Rheinsberg, Rafael Passos, Benjamin Tissoires, Jiri Kosina
Cc: Shuah Khan, Brigham Campbell, Jori Koolstra, linux-input
In-Reply-To: <850e8696-311e-45e4-874a-8089afe35907@app.fastmail.com>
On Mon Jul 13, 2026 at 6:28 AM -03, David Rheinsberg wrote:
> What device name are you referring to exactly?
I was looking at the `wiimote_devtype_names` table,
but I figured out its only used for logging.
What I actually wanted, was a way to return a propper name to the user,
with a controller/player ID.
When listing bluetooth devices, I have 4 entries like the following:
`Device <UNIQUE MAC> Nintendo RVL-CNT-01-TR` (x4)
I wanted to display a friendly name to the user, like the one on dmesg:
`Nintendo Wii Remote Plus (Gen 2) (1)` (where 1 is the player id).
If you think it makes sense, and its possible, I would love to make
this change in this or a future set :)
> Anyway, I like the series. I will review your v2 properly, so please go ahead!
Thanks, will do!
Rafael Passos
^ permalink raw reply
* Re: [PATCH v2] HID: playstation: Support DualSense player LED brightness control
From: sashiko-bot @ 2026-07-14 19:18 UTC (permalink / raw)
To: k8ie; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260714184225.84646-1-k8ie@mcld.eu>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing cancel_work_sync() in the probe error paths of dualsense_create() and dualshock4_create() causes a use-after-free vulnerability.
--
--- Patch [1]: [PATCH v2] HID: playstation: Support DualSense player LED brightness control ---
commit c4af8f1ee7d50867512c691be4fbde7636ea1510
Author: Kateřina Medvědová <k8ie@mcld.eu>
HID: playstation: Support DualSense player LED brightness control
This patch wires up the controls for the brightness byte on the DualSense
player indicator LEDs. It increases the max_brightness of each player's LED
classdev from 1 to 3, and updates the brightness values before sending
the output reports.
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index f9dc9378cf777..58786117ec016 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
[ ... ]
> @@ -1742,15 +1776,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
> int i, ret;
>
> static const struct ps_led_info player_leds_info[] = {
> - { LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
> + { LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
> dualsense_player_led_set_brightness },
> - { LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
> + { LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
> dualsense_player_led_set_brightness },
[Severity: Critical]
This is a pre-existing issue, but the error paths for dualsense_create() and
dualshock4_create() appear to be missing a cancel_work_sync() call.
If initialization fails after the work is scheduled (for example, if
ps_led_register() fails in dualsense_create()), the code jumps to the err
path:
drivers/hid/hid-playstation.c:dualsense_create() {
...
err:
ps_devices_list_remove(ps_dev);
return ERR_PTR(ret);
}
Since ds->output_worker is not canceled before the devm routines free the
ds/ds4 structure, could this result in a use-after-free when the global
workqueue eventually executes the worker function?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714184225.84646-1-k8ie@mcld.eu?part=1
^ permalink raw reply
* Re: [PATCH v2 4/6] tools/build: Allow versioning LLVM readelf
From: Ihor Solodrai @ 2026-07-14 18:49 UTC (permalink / raw)
To: James Clark
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jiri Kosina, Benjamin Tissoires,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Andrii Nakryiko, Eduard Zingerman,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song
In-Reply-To: <a8d4ab9b-b2e3-407c-9898-1b9f7339f448@linaro.org>
On 7/14/26 9:41 AM, James Clark wrote:
>
>
> On 10/07/2026 23:29, Ihor Solodrai wrote:
>> On 5/18/26 2:03 AM, James Clark wrote:
>>> Documentation/kbuild/llvm.rst mentions that readelf is included in the
>>> LLVM toolchain, but it's not currently included in this block.
>>>
>>> Add it so that LLVM=... options also apply to readelf. Users in tools/
>>> were Perf which was hardcoding it, and another was the BPF makefile.
>>> Both already include Makefile.include so convert them to use the new
>>> variable.
>>>
>>> It also didn't have the cross compile prefix, so either readelf didn't
>>> mind opening cross binaries, or it wasn't working for cross builds.
>>
>> I'm pretty sure it's the former. readelf/llvm-readelf are only used
>> libbpf makefile to read ELF symbol tables, which should be arch-independent.
>>
>> We've been cross-compiling the kernel (and libbpf) on BPF CI for a
>> long time. So the unprefixed readelf is already working, and adding
>
> I think "working" isn't technically correct when you take into account the documented behavior of the versioned LLVM= option [1]. You would get one version of the toolchain used for some of the build and a different version used here. That's what caused the build failure in Perf that resulted in the tidyup, because they're not always compatible.
>
> [1]: Documentation/kbuild/llvm.rst
>
>> the $(CROSS_COMPILE) prefix only adds a new requirement.
>>
>> I don't think this change in tools/lib/bpf/Makefile is a good idea,
>> we could potentially break some environments. Even though hardcoded
>> readelf doesn't look nice.
>>
>>
>
> IMO it's worth the risk for the cleanup, it also helps to stop propagating it with future copy pastes.
>
> Do you think it's likely that someone has CROSS_COMPILE set but doesn't have that readelf installed? Installing gcc-aarch64-linux-gnu on Debian/Ubuntu gives you aarch64-linux-gnu-readelf, so it would have to be a unique setup to not have it.
Fair points.
Still, here readelf only enumerates symbol names from already-built
objects to count them, so a version or arch mismatch shouldn't matter,
and didn't so far.
>
> It sounds like maybe you want this bpf makefile to explicitly use 'HOSTREADELF', if you're really sure it never requires arch specific stuff? If we added that it would fit better with the rest of the cleanup. But I think 'READELF' is less confusing and take the risk of some build breakages.
Yes, that's a good idea.
So maybe something like this in Makefile.include:
# if LLVM
$(call allow-override,HOSTREADELF,$(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX))
# else
$(call allow-override,HOSTREADELF,readelf)
and use $(HOSTREADELF) in tools/lib/bpf/Makefile.
Thanks.
>
>>>
>>> Signed-off-by: James Clark <james.clark@linaro.org>
>>> ---
>>> tools/lib/bpf/Makefile | 8 ++++----
>>> tools/perf/Makefile.perf | 1 -
>>> tools/scripts/Makefile.include | 2 ++
>>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>>> index 168140f8e646..180dca9c57c8 100644
>>> --- a/tools/lib/bpf/Makefile
>>> +++ b/tools/lib/bpf/Makefile
>>> @@ -114,12 +114,12 @@ PC_FILE := $(addprefix $(OUTPUT),$(PC_FILE))
>>> TAGS_PROG := $(if $(shell which etags 2>/dev/null),etags,ctags)
>>> -GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
>>> +GLOBAL_SYM_COUNT = $(shell $(READELF) -s --wide $(BPF_IN_SHARED) | \
>>> cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
>>> sed 's/\[.*\]//' | \
>>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
>>> sort -u | wc -l)
>>> -VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
>>> +VERSIONED_SYM_COUNT = $(shell $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
>>> sed 's/\[.*\]//' | \
>>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
>>> grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
>>> @@ -182,12 +182,12 @@ check_abi: $(OUTPUT)libbpf.so $(VERSION_SCRIPT)
>>> "versioned symbols in $^ ($(VERSIONED_SYM_COUNT))." \
>>> "Please make sure all LIBBPF_API symbols are" \
>>> "versioned in $(VERSION_SCRIPT)." >&2; \
>>> - readelf -s --wide $(BPF_IN_SHARED) | \
>>> + $(READELF) -s --wide $(BPF_IN_SHARED) | \
>>> cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
>>> sed 's/\[.*\]//' | \
>>> awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'| \
>>> sort -u > $(OUTPUT)libbpf_global_syms.tmp; \
>>> - readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
>>> + $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
>>
>>> sed 's/\[.*\]//' | \
>>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}'| \
>>> grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | \
>>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>>> index 0aba14f22a06..63276bf55856 100644
>>> --- a/tools/perf/Makefile.perf
>>> +++ b/tools/perf/Makefile.perf
>>> @@ -215,7 +215,6 @@ FLEX ?= flex
>>> BISON ?= bison
>>> STRIP = strip
>>> AWK = awk
>>> -READELF ?= readelf
>>> # include Makefile.config by default and rule out
>>> # non-config cases
>>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
>>> index e81e5b479c56..380ad84ac51e 100644
>>> --- a/tools/scripts/Makefile.include
>>> +++ b/tools/scripts/Makefile.include
>>> @@ -73,6 +73,7 @@ ifneq ($(LLVM),)
>>> $(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
>>> $(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
>>> $(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
>>> + $(call allow-override,READELF,$(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX))
>>> else
>>> # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
>>> $(call allow-override,CC,$(CROSS_COMPILE)gcc)
>>> @@ -80,6 +81,7 @@ else
>>> $(call allow-override,LD,$(CROSS_COMPILE)ld)
>>> $(call allow-override,CXX,$(CROSS_COMPILE)g++)
>>> $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
>>> + $(call allow-override,READELF,$(CROSS_COMPILE)readelf)
>>> # Host versions aren't prefixed
>>> $(call allow-override,HOSTAR,ar)
>>>
>>
>
^ permalink raw reply
* [PATCH v2] HID: playstation: Support DualSense player LED brightness control
From: k8ie @ 2026-07-14 18:42 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Kateřina Medvědová, Jiri Kosina,
Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260714180424.70662-1-k8ie@mcld.eu>
From: Kateřina Medvědová <k8ie@mcld.eu>
The DualSense's 5 player indicator LEDs are currently only exposed as
plain on/off LED classdevs (max_brightness = 1). The controller
firmware also supports a global brightness level (high/medium/low)
for whichever player LEDs are currently lit. The driver already
defines the brightness byte but never uses it
(dualsense_output_report_common::led_brightness).
This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted.
Increase max_brightness of each player's LED classdev from 1 to 3.
brightness = 0 still turns an LED off, writing brightness = 1..3
turns the LED on and applies the brightness to all lit player LEDs.
This is a hardware limitation and the driver reflects that by
keeping the sysfs brightness of player LEDs in sync. The last
brightness write wins.
The firmware encoding is inverted (lower is brighter), so map LED
core brightness values to the device-specific encoding before
sending reports.
Also set DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE
whenever player LED state is updated so firmware applies the
brightness byte.
Existing userspace that writes brightness=1 as a simple "on" value
continues to work, but LEDs will now be lit at the lowest brightness
instead of the highest brightness.
Tested on a DualSense controller (product ID 0x0ce6) using Bluetooth
and USB.
Assisted-by: OpenCode:claude-sonnet-5
Signed-off-by: Kateřina Medvědová <k8ie@mcld.eu>
---
First-time contributing a patch.
v1 -> v2: Fixed a data race flagged by the list's automated review bot:
dualsense_player_led_get_brightness() read player_leds_state and
player_leds_brightness without holding ds->base.lock, while the setter
updates both fields together under that lock. Now takes the same lock
in the getter.
The bot also flagged a pre-existing missing cancel_work_sync() in the
dualsense_create()/dualshock4_create() error paths. That is unrelated
to this change (present before this patch) and is intentionally left
unfixed.
---
drivers/hid/hid-playstation.c | 50 +++++++++++++++++++++++++++++------
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f9dc9378cf77..58786117ec01 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -164,11 +164,20 @@ struct ps_led_info {
#define DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE BIT(7)
#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
+#define DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE BIT(0)
#define DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL GENMASK(5, 4)
#define DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN GENMASK(2, 0)
#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
+/*
+ * Player LED brightness levels. Lower values are brighter; this is inverted
+ * from the LED subsystem's convention where higher values mean brighter.
+ */
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_HIGH 0
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_MEDIUM 1
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW 2
+
/* DualSense hardware limits */
#define DS_ACC_RES_PER_G 8192
#define DS_ACC_RANGE (4 * DS_ACC_RES_PER_G)
@@ -225,6 +234,7 @@ struct dualsense {
/* Player leds */
bool update_player_leds;
u8 player_leds_state;
+ u8 player_leds_brightness;
struct led_classdev player_leds[5];
struct work_struct output_worker;
@@ -1219,12 +1229,24 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
return 0;
}
+/*
+ * The DualSense's player LEDs only support a single, shared brightness level
+ * for all lit LEDs -- there is no per-LED brightness control. We still expose
+ * per-LED on/off state plus 3 brightness levels through each LED classdev's
+ * max_brightness of 3; the last value written by any player LED classdev sets
+ * the shared level for all of them.
+ */
static enum led_brightness dualsense_player_led_get_brightness(struct led_classdev *led)
{
struct hid_device *hdev = to_hid_device(led->dev->parent);
struct dualsense *ds = hid_get_drvdata(hdev);
- return !!(ds->player_leds_state & BIT(led - ds->player_leds));
+ guard(spinlock_irqsave)(&ds->base.lock);
+
+ if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
+ return LED_OFF;
+
+ return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;
}
static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
@@ -1235,10 +1257,18 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
scoped_guard(spinlock_irqsave, &ds->base.lock) {
led_index = led - ds->player_leds;
- if (value == LED_OFF)
+ if (value == LED_OFF) {
ds->player_leds_state &= ~BIT(led_index);
- else
+ } else {
ds->player_leds_state |= BIT(led_index);
+ /* Convert Linux brightness (1=dimmest) to firmware
+ * scale (0=brightest), clamping to the valid range.
+ */
+ value = clamp_t(enum led_brightness, value, 1,
+ DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1);
+ ds->player_leds_brightness =
+ DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - value;
+ }
ds->update_player_leds = true;
}
@@ -1355,6 +1385,10 @@ static void dualsense_output_worker(struct work_struct *work)
DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE;
common->player_leds = ds->player_leds_state;
+ common->valid_flag2 |=
+ DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE;
+ common->led_brightness = ds->player_leds_brightness;
+
ds->update_player_leds = false;
}
@@ -1742,15 +1776,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
int i, ret;
static const struct ps_led_info player_leds_info[] = {
- { LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER3, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER4, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness }
};
--
2.55.0
^ permalink raw reply related
* Re: [PATCH] HID: playstation: Support DualSense player LED brightness control
From: sashiko-bot @ 2026-07-14 18:25 UTC (permalink / raw)
To: Kateřina Medvědová; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260714180424.70662-1-k8ie@mcld.eu>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Multi-variable data race on `ds->player_leds_state` and `ds->player_leds_brightness` in `dualsense_player_led_get_brightness()`.
Pre-existing issues:
- [High] Missing workqueue cancellation in the `dualsense_create()` and `dualshock4_create()` error paths causes a use-after-free if probe fails.
--
--- Patch [1]: [PATCH] HID: playstation: Support DualSense player LED brightness control ---
commit 9dd8a497081a6ac6ae8e211e738b3ef23a0669be
Author: Kateřina Medvědová <k8ie@mcld.eu>
HID: playstation: Support DualSense player LED brightness control
This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted. It increases the
max_brightness of each player's LED classdev from 1 to 3, and correctly
updates the firmware encoding.
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index f9dc9378cf777..78e43a176248a 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
[ ... ]
> @@ -1219,12 +1229,22 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
> return 0;
> }
>
> +/*
> + * The DualSense's player LEDs only support a single, shared brightness level
> + * for all lit LEDs -- there is no per-LED brightness control. We still expose
> + * per-LED on/off state plus 3 brightness levels through each LED classdev's
> + * max_brightness of 3; the last value written by any player LED classdev sets
> + * the shared level for all of them.
> + */
> static enum led_brightness dualsense_player_led_get_brightness(struct led_classdev *led)
> {
> struct hid_device *hdev = to_hid_device(led->dev->parent);
> struct dualsense *ds = hid_get_drvdata(hdev);
>
> - return !!(ds->player_leds_state & BIT(led - ds->player_leds));
> + if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
> + return LED_OFF;
> +
> + return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;
[Severity: Medium]
Could this lockless read result in a multi-variable data race?
In dualsense_player_led_set_brightness(), ds->player_leds_state and
ds->player_leds_brightness are updated together under ds->base.lock.
If dualsense_player_led_get_brightness() reads them without the lock,
a concurrent read could observe the updated state but the old
brightness (or vice-versa due to compiler or CPU reordering), returning
an inconsistent value to userspace.
> }
>
> static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
[ ... ]
> @@ -1742,15 +1774,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
> int i, ret;
>
> static const struct ps_led_info player_leds_info[] = {
> - { LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
> + { LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
> dualsense_player_led_set_brightness },
[ ... ]
> - { LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
> + { LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
> dualsense_player_led_set_brightness }
> };
>
[Severity: High]
This is a pre-existing issue, but missing workqueue cancellation in the
dualsense_create() and dualshock4_create() error paths could cause a
use-after-free if probe fails.
During probe, dualsense_set_lightbar() is called to set the initial lightbar
color, which implicitly schedules ds->output_worker:
drivers/hid/hid-playstation.c:dualsense_create() {
...
dualsense_set_lightbar(ds, 0, 0, 128); /* blue */
for (i = 0; i < ARRAY_SIZE(player_leds_info); i++) {
const struct ps_led_info *led_info = &player_leds_info[i];
ret = ps_led_register(ps_dev, &ds->player_leds[i], led_info);
if (ret < 0)
goto err;
}
...
}
If a subsequent initialization step fails, the error path returns and allows
devres to free the allocated ds struct:
err:
ps_devices_list_remove(ps_dev);
return ERR_PTR(ret);
}
Because the worker is not cancelled, it remains in the system workqueue and
will dereference the freed ds pointer when it executes. Does the error path
need an explicit cancel_work_sync(&ds->output_worker) to prevent this? The
exact same vulnerability exists in dualshock4_create() due to
dualshock4_set_bt_poll_interval().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714180424.70662-1-k8ie@mcld.eu?part=1
^ permalink raw reply
* Re: [PATCH v2] HID: uclogic: fix use-after-free of inrange_timer on remove
From: sashiko-bot @ 2026-07-14 18:10 UTC (permalink / raw)
To: Ibrahim Hashimov; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260714173946.73773-1-security@auditcode.ai>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Multiple HID drivers suffer from race conditions during device removal due to incorrect or missing timer teardown.
- [High] In `hid-sony.c`, `sony_remove()` fails to synchronously kill the `ghl_urb` before freeing it, leading to a Use-After-Free in the URB callback.
--
--- Patch [1]: [PATCH v2] HID: uclogic: fix use-after-free of inrange_timer on remove ---
commit b8f98aaaf048d9d412a83d45b98e8c080f6ef05c
Author: Ibrahim Hashimov <security@auditcode.ai>
HID: uclogic: fix use-after-free of inrange_timer on remove
uclogic_remove() cancels the pen in-range timer and then stops the
device. A report landing between the timer_delete_sync() call and the transport
teardown in hid_hw_stop() re-arms inrange_timer after it was cancelled.
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..0b8a83fa6c5be 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -548,7 +548,17 @@ static void uclogic_remove(struct hid_device *hdev)
> {
> struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
>
> - timer_delete_sync(&drvdata->inrange_timer);
> + /*
> + * Shut the in-range timer down before stopping the device.
> + * uclogic_raw_event_pen() re-arms inrange_timer on every pen report
> + * and keeps running until hid_hw_stop() stops the transport, so a
> + * plain timer_delete_sync() here can be undone by a report landing in
> + * the window before hid_hw_stop(). timer_shutdown_sync() cancels the
> + * timer and makes any later re-arm a no-op, so it is provably dead
> + * before hid_hw_stop() frees the input device drvdata->pen_input
> + * points at.
> + */
> + timer_shutdown_sync(&drvdata->inrange_timer);
> hid_hw_stop(hdev);
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this
fix for the teardown race in uclogic, I noticed multiple other HID drivers
suffer from similar pre-existing race conditions during device removal.
For instance, drivers/hid/hid-letsketch.c lacks a .remove callback entirely
in letsketch_driver:
static struct hid_driver letsketch_driver = {
.name = "letsketch",
.id_table = letsketch_devices,
.probe = letsketch_probe,
.raw_event = letsketch_raw_event,
};
This means the inrange_timer is never explicitly canceled and can outlive
the driver's devres cleanup.
In drivers/hid/hid-multitouch.c mt_remove() and drivers/hid/hid-prodikeys.c
pk_remove(), the code calls timer_delete_sync() before hid_hw_stop(), leaving
the exact same window where incoming reports can re-arm the timer.
Additionally, in drivers/hid/hid-appleir.c appleir_remove() calls
hid_hw_stop() before timer_delete_sync(). Because hid_hw_stop() frees the
hid-input device, this can cause a use-after-free if the timer fires in this
window.
Would it make sense to apply this same timer_shutdown_sync() pattern and
add the missing remove callbacks to these other HID drivers as well?
[Severity: High]
This is also a pre-existing issue, but while looking at these removal paths,
I spotted another teardown race in drivers/hid/hid-sony.c during sony_remove().
The cleanup sequence is:
timer_delete_sync(&sc->ghl_poke_timer);
usb_free_urb(sc->ghl_urb);
If ghl_poke_timer fires right before it is canceled, it submits ghl_urb.
The usb_free_urb() call only decrements the reference count; it does not
cancel an in-flight transfer.
When the URB later completes, ghl_magic_poke_cb() will dereference the
freed sc pointer to re-arm the timer, leading to a use-after-free.
Should this code be using usb_kill_urb() prior to freeing the URB to
ensure any in-flight transfers are synchronously terminated?
> kfree(drvdata->desc_ptr);
> uclogic_params_cleanup(&drvdata->params);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714173946.73773-1-security@auditcode.ai?part=1
^ permalink raw reply
* [PATCH] HID: playstation: Support DualSense player LED brightness control
From: Kateřina Medvědová @ 2026-07-14 18:04 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Kateřina Medvědová, Jiri Kosina,
Benjamin Tissoires, linux-input, linux-kernel
The DualSense's 5 player indicator LEDs are currently only exposed as
plain on/off LED classdevs (max_brightness = 1). The controller
firmware also supports a global brightness level (high/medium/low)
for whichever player LEDs are currently lit. The driver already
defines the brightness byte but never uses it
(dualsense_output_report_common::led_brightness).
This patch wires up the controls for the brightness byte, allowing
the player indicator brightness to be adjusted.
Increase max_brightness of each player's LED classdev from 1 to 3.
brightness = 0 still turns an LED off, writing brightness = 1..3
turns the LED on and applies the brightness to all lit player LEDs.
This is a hardware limitation and the driver reflects that by
keeping the sysfs brightness of player LEDs in sync. The last
brightness write wins.
The firmware encoding is inverted (lower is brighter), so map LED
core brightness values to the device-specific encoding before
sending reports.
Also set DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE
whenever player LED state is updated so firmware applies the
brightness byte.
Existing userspace that writes brightness=1 as a simple "on" value
continues to work, but LEDs will now be lit at the lowest brightness
instead of the highest brightness.
Tested on a DualSense controller (product ID 0x0ce6) using Bluetooth
and USB.
Assisted-by: OpenCode:claude-sonnet-5
Signed-off-by: Kateřina Medvědová <k8ie@mcld.eu>
---
First-time contributing a patch.
---
drivers/hid/hid-playstation.c | 48 +++++++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f9dc9378cf77..78e43a176248 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -164,11 +164,20 @@ struct ps_led_info {
#define DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE BIT(7)
#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
+#define DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE BIT(0)
#define DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL GENMASK(5, 4)
#define DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN GENMASK(2, 0)
#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
+/*
+ * Player LED brightness levels. Lower values are brighter; this is inverted
+ * from the LED subsystem's convention where higher values mean brighter.
+ */
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_HIGH 0
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_MEDIUM 1
+#define DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW 2
+
/* DualSense hardware limits */
#define DS_ACC_RES_PER_G 8192
#define DS_ACC_RANGE (4 * DS_ACC_RES_PER_G)
@@ -225,6 +234,7 @@ struct dualsense {
/* Player leds */
bool update_player_leds;
u8 player_leds_state;
+ u8 player_leds_brightness;
struct led_classdev player_leds[5];
struct work_struct output_worker;
@@ -1219,12 +1229,22 @@ static int dualsense_lightbar_set_brightness(struct led_classdev *cdev,
return 0;
}
+/*
+ * The DualSense's player LEDs only support a single, shared brightness level
+ * for all lit LEDs -- there is no per-LED brightness control. We still expose
+ * per-LED on/off state plus 3 brightness levels through each LED classdev's
+ * max_brightness of 3; the last value written by any player LED classdev sets
+ * the shared level for all of them.
+ */
static enum led_brightness dualsense_player_led_get_brightness(struct led_classdev *led)
{
struct hid_device *hdev = to_hid_device(led->dev->parent);
struct dualsense *ds = hid_get_drvdata(hdev);
- return !!(ds->player_leds_state & BIT(led - ds->player_leds));
+ if (!(ds->player_leds_state & BIT(led - ds->player_leds)))
+ return LED_OFF;
+
+ return DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - ds->player_leds_brightness;
}
static int dualsense_player_led_set_brightness(struct led_classdev *led, enum led_brightness value)
@@ -1235,10 +1255,18 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
scoped_guard(spinlock_irqsave, &ds->base.lock) {
led_index = led - ds->player_leds;
- if (value == LED_OFF)
+ if (value == LED_OFF) {
ds->player_leds_state &= ~BIT(led_index);
- else
+ } else {
ds->player_leds_state |= BIT(led_index);
+ /* Convert Linux brightness (1=dimmest) to firmware
+ * scale (0=brightest), clamping to the valid range.
+ */
+ value = clamp_t(enum led_brightness, value, 1,
+ DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1);
+ ds->player_leds_brightness =
+ DS_OUTPUT_PLAYER_LED_BRIGHTNESS_LOW + 1 - value;
+ }
ds->update_player_leds = true;
}
@@ -1355,6 +1383,10 @@ static void dualsense_output_worker(struct work_struct *work)
DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE;
common->player_leds = ds->player_leds_state;
+ common->valid_flag2 |=
+ DS_OUTPUT_VALID_FLAG2_LED_BRIGHTNESS_CONTROL_ENABLE;
+ common->led_brightness = ds->player_leds_brightness;
+
ds->update_player_leds = false;
}
@@ -1742,15 +1774,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
int i, ret;
static const struct ps_led_info player_leds_info[] = {
- { LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER1, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER2, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER3, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER4, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness },
- { LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
+ { LED_FUNCTION_PLAYER5, "white", 3, dualsense_player_led_get_brightness,
dualsense_player_led_set_brightness }
};
--
2.55.0
^ permalink raw reply related
* [PATCH v2] HID: uclogic: fix use-after-free of inrange_timer on remove
From: Ibrahim Hashimov @ 2026-07-14 17:39 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, stable
In-Reply-To: <20260713121042.2321-1-security@auditcode.ai>
uclogic_remove() cancels the pen in-range timer and then stops the
device:
timer_delete_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
timer_delete_sync() only guarantees the timer is idle at that instant.
uclogic_raw_event_pen() keeps delivering pen reports until hid_hw_stop()
stops the transport several lines later, and every report with
pen->inrange == UCLOGIC_PARAMS_PEN_INRANGE_NONE re-arms the timer:
mod_timer(&drvdata->inrange_timer, jiffies + msecs_to_jiffies(100));
A report landing between the timer_delete_sync() call and the transport
teardown in hid_hw_stop() re-arms inrange_timer after it was cancelled.
uclogic_remove() then returns and the devm drvdata is freed, while
hid_hw_stop() has already freed the input device drvdata->pen_input
points at, so when the timer fires ~100 ms later
uclogic_inrange_timeout() dereferences freed memory -- a use-after-free
in timer-softirq context.
Swapping the two calls is not a fix: stopping the device first frees
drvdata->pen_input via hidinput_disconnect() while the timer may still
be pending, so a timer already armed before removal fires on the freed
input device in the window before timer_delete_sync() runs.
Use timer_shutdown_sync() before hid_hw_stop() instead. It cancels the
timer, waits for a running callback while pen_input is still valid, and
prevents any further re-arming -- a later mod_timer() from an in-flight
report is silently ignored -- so the timer is provably dead before
hid_hw_stop() frees the inputs. This is the ordering the timer core
documents for this "timer re-armed from another path" teardown case.
Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: v1 reordered hid_hw_stop() before timer_delete_sync(), which traded
the re-arm use-after-free for one on the freed input device (thanks
to sashiko-bot for spotting it). Keep the original order and use
timer_shutdown_sync() to disarm the timer, which closes the re-arm
race without ever touching pen_input after it is freed.
drivers/hid/hid-uclogic-core.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..0b8a83fa6c5b 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -548,7 +548,17 @@ static void uclogic_remove(struct hid_device *hdev)
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- timer_delete_sync(&drvdata->inrange_timer);
+ /*
+ * Shut the in-range timer down before stopping the device.
+ * uclogic_raw_event_pen() re-arms inrange_timer on every pen report
+ * and keeps running until hid_hw_stop() stops the transport, so a
+ * plain timer_delete_sync() here can be undone by a report landing in
+ * the window before hid_hw_stop(). timer_shutdown_sync() cancels the
+ * timer and makes any later re-arm a no-op, so it is provably dead
+ * before hid_hw_stop() frees the input device drvdata->pen_input
+ * points at.
+ */
+ timer_shutdown_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
kfree(drvdata->desc_ptr);
uclogic_params_cleanup(&drvdata->params);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth
From: Alec Hall @ 2026-07-14 17:17 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Alec Hall
In-Reply-To: <20260714102540.3EB2E1F000E9@smtp.kernel.org>
> Sashiko AI review found 2 potential issue(s) to consider:
> - [High] NULL dereference of `msc->input` in magicmouse_raw_event /
> magicmouse_emit_touch
> - [High] Unbounded recursion via nested DOUBLE_REPORT_ID (0xf7) reports
Thanks for the review. Both are pre-existing and are neither introduced nor
worsened by this series, but they're real and I'm happy to fix them.
> can this function trigger a kernel panic due to a NULL pointer dereference
> on msc->input?
This series does not add any new use of msc->input. The battery report path
added in patch 3 returns through the switch "default" case (report 0x90 is
not a touch report), i.e. before the input->id.product access at the end of
magicmouse_raw_event(), and magicmouse_report_charge_status() only calls
hid_get_battery() -- it never touches msc->input.
That said, the pre-existing exposure you describe (a device that takes the
early return in magicmouse_probe() and then sends a touch-shaped report) is
real, and an early "if (!msc->input) return 0;" at the top of
magicmouse_raw_event() is a sensible guard.
> does this function have a potential stack overflow via unbounded recursion?
Also untouched by this series, but yes, the self-recursion on DOUBLE_REPORT_ID
is unbounded on malicious input and worth bounding (reworking it into a loop,
or capping the nesting depth).
Since both are independent of the battery feature, I'd prefer to send them as
a separate hardening series rather than fold them into this one. Dmitry, would
you like me to post that now, or do you consider these out of scope here? I'm
also happy to add just the msc->input guard to this series if you'd rather see
it alongside patch 3, since that patch touches magicmouse_raw_event().
Thanks,
Alec Hall
^ permalink raw reply
* Re: [PATCH v2 4/6] tools/build: Allow versioning LLVM readelf
From: James Clark @ 2026-07-14 16:41 UTC (permalink / raw)
To: Ihor Solodrai
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jiri Kosina, Benjamin Tissoires,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Andrii Nakryiko, Eduard Zingerman,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song
In-Reply-To: <c366293c-7bcd-4e28-b3c4-575203403489@linux.dev>
On 10/07/2026 23:29, Ihor Solodrai wrote:
> On 5/18/26 2:03 AM, James Clark wrote:
>> Documentation/kbuild/llvm.rst mentions that readelf is included in the
>> LLVM toolchain, but it's not currently included in this block.
>>
>> Add it so that LLVM=... options also apply to readelf. Users in tools/
>> were Perf which was hardcoding it, and another was the BPF makefile.
>> Both already include Makefile.include so convert them to use the new
>> variable.
>>
>> It also didn't have the cross compile prefix, so either readelf didn't
>> mind opening cross binaries, or it wasn't working for cross builds.
>
> I'm pretty sure it's the former. readelf/llvm-readelf are only used
> libbpf makefile to read ELF symbol tables, which should be arch-independent.
>
> We've been cross-compiling the kernel (and libbpf) on BPF CI for a
> long time. So the unprefixed readelf is already working, and adding
I think "working" isn't technically correct when you take into account
the documented behavior of the versioned LLVM= option [1]. You would get
one version of the toolchain used for some of the build and a different
version used here. That's what caused the build failure in Perf that
resulted in the tidyup, because they're not always compatible.
[1]: Documentation/kbuild/llvm.rst
> the $(CROSS_COMPILE) prefix only adds a new requirement.
>
> I don't think this change in tools/lib/bpf/Makefile is a good idea,
> we could potentially break some environments. Even though hardcoded
> readelf doesn't look nice.
>
>
IMO it's worth the risk for the cleanup, it also helps to stop
propagating it with future copy pastes.
Do you think it's likely that someone has CROSS_COMPILE set but doesn't
have that readelf installed? Installing gcc-aarch64-linux-gnu on
Debian/Ubuntu gives you aarch64-linux-gnu-readelf, so it would have to
be a unique setup to not have it.
It sounds like maybe you want this bpf makefile to explicitly use
'HOSTREADELF', if you're really sure it never requires arch specific
stuff? If we added that it would fit better with the rest of the
cleanup. But I think 'READELF' is less confusing and take the risk of
some build breakages.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> tools/lib/bpf/Makefile | 8 ++++----
>> tools/perf/Makefile.perf | 1 -
>> tools/scripts/Makefile.include | 2 ++
>> 3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index 168140f8e646..180dca9c57c8 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -114,12 +114,12 @@ PC_FILE := $(addprefix $(OUTPUT),$(PC_FILE))
>>
>> TAGS_PROG := $(if $(shell which etags 2>/dev/null),etags,ctags)
>>
>> -GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
>> +GLOBAL_SYM_COUNT = $(shell $(READELF) -s --wide $(BPF_IN_SHARED) | \
>> cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
>> sed 's/\[.*\]//' | \
>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
>> sort -u | wc -l)
>> -VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
>> +VERSIONED_SYM_COUNT = $(shell $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
>> sed 's/\[.*\]//' | \
>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
>> grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
>> @@ -182,12 +182,12 @@ check_abi: $(OUTPUT)libbpf.so $(VERSION_SCRIPT)
>> "versioned symbols in $^ ($(VERSIONED_SYM_COUNT))." \
>> "Please make sure all LIBBPF_API symbols are" \
>> "versioned in $(VERSION_SCRIPT)." >&2; \
>> - readelf -s --wide $(BPF_IN_SHARED) | \
>> + $(READELF) -s --wide $(BPF_IN_SHARED) | \
>> cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
>> sed 's/\[.*\]//' | \
>> awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'| \
>> sort -u > $(OUTPUT)libbpf_global_syms.tmp; \
>> - readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
>> + $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
>
>> sed 's/\[.*\]//' | \
>> awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}'| \
>> grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | \
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index 0aba14f22a06..63276bf55856 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -215,7 +215,6 @@ FLEX ?= flex
>> BISON ?= bison
>> STRIP = strip
>> AWK = awk
>> -READELF ?= readelf
>>
>> # include Makefile.config by default and rule out
>> # non-config cases
>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
>> index e81e5b479c56..380ad84ac51e 100644
>> --- a/tools/scripts/Makefile.include
>> +++ b/tools/scripts/Makefile.include
>> @@ -73,6 +73,7 @@ ifneq ($(LLVM),)
>> $(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
>> $(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
>> $(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
>> + $(call allow-override,READELF,$(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX))
>> else
>> # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
>> $(call allow-override,CC,$(CROSS_COMPILE)gcc)
>> @@ -80,6 +81,7 @@ else
>> $(call allow-override,LD,$(CROSS_COMPILE)ld)
>> $(call allow-override,CXX,$(CROSS_COMPILE)g++)
>> $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
>> + $(call allow-override,READELF,$(CROSS_COMPILE)readelf)
>>
>> # Host versions aren't prefixed
>> $(call allow-override,HOSTAR,ar)
>>
>
^ permalink raw reply
* Re: [PATCH 0/2] input: fsl,scu-key: Add compatible string fsl,imx8qm-sc-key
From: Frank.Li @ 2026-07-14 15:47 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dong Aisheng, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Frank Li
Cc: linux-input, devicetree, linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20251028-b4_qm_scu_key-v1-0-9732e92a5e83@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
On Tue, 28 Oct 2025 16:01:27 -0400, Frank Li wrote:
> bindings: add fsl,imx8qm-sc-key.
> dts: add fsl,imx8qm-sc-key support.
Applied, thanks!
[1/2] dt-bindings: input: fsl,scu-key: Add compatible string fsl,imx8qm-sc-key
commit: df9b9d59586f3d8bdeba5b3c7b7ea68cb0302fb7
I picked it because it was missed since 2025 OCT and ping 2026 Jan and
fsl,scu-key.yaml was only used by imx/
[2/2] arm64: dts: imx8qm: add scu power key support
commit: f9c8aaffc782cae682d3606037f2d02fb4e0035a
Best regards,
--
Frank Li <Frank.Li@nxp.com>
^ permalink raw reply
* Re: [PATCH] Input: zinitix: iterate contact slots by finger count
From: timon @ 2026-07-14 15:35 UTC (permalink / raw)
To: Thành Nguyễn, nikita
Cc: linusw, dmitry.torokhov, linux-input, linux-kernel, linus.walleij
In-Reply-To: <CAPWvNi7ATfEKCC6qn+DTHnSLJxXamYiiNWA+1JgQmw2RG+PH6g@mail.gmail.com>
Sorry for not replying earlier (life), but I don't think the v2 patch
will fully work correctly.
At least on my samsung a3 the loop needs to work as it did before
e941dc13fd37.
Meaning: iterate over all slots and check the (sub_status &
SUB_BIT_EXIST) for whether the finger is active or not.
Because: the active fingers can be in later slots with the earlier slots
being inactive.
For example when the user:
1. touches with one finger (1 finger active in slot 0)
2. touches somewhere else with a second finger (2 fingers active in
slots 0, 1)
3. lets go with the first finger (1 finger active in slot 1)
In such a case the loop would only iterate over slot 0 and not report
any fingers despite slot 1 still being active.
In practice this will likely be usable... but afaik and can tell it's
still incorrect behaviour.
Best Regards,
Timon
On 7/5/26 13:17, Thành Nguyễn wrote:
> Hi Nikita, Linus, and Dmitry,
>
> Thank you all for the feedback and for confirming the fix on your end, Nikita.
>
> Unfortunately, since it's been a while since I submitted the v2 patch,
> I recently had to completely wipe my development machine and no longer
> have the local environment or the original patch files to create a v3.
>
> Since the logic in v2 is sound and the only change needed is a brief
> comment explaining the dual mask/count nature of `finger_mask`, would
> one of you be willing to add that comment and apply the patch on my
> behalf? I'd really appreciate it.
>
> Thanks again for your time and help with getting this merged!
>
> Best regards,
> Thanh Nguyen
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
From: Erik Håkansson @ 2026-07-14 15:33 UTC (permalink / raw)
To: Kateřina Medvědová
Cc: bentiss, hadess, jikos, lains, linux-input, linux-kernel
In-Reply-To: <54844d1b-636c-4896-8994-f4e960e57944@mcld.eu>
On 7/14/26 12:32, Kateřina Medvědová wrote:
> Hi,
>
> Just tested V1 again with both of the receiver versions I have on hand
> and the input issues I described happen on both.
I see. Then I assume it either comes down to our different keyboard
models behaving differently, or something behaving differently in our
setups. I use KDE Plasma on Fedora for what it's worth.
>
> btw, thank you for pointing out that you can check the FW version in
> fwupd. This greatly simplified testing.
>
> I do have one find -- when a device gets powered off (yanked battery,
> switch moved to the off position, switching devices), the battery
> stays in upower. Unifying also does this, disconnected devices stay.
> Either this was intentional or a bug. Either way the behavior is
> consistent with the existing code and isn't introduced by your
> changes. I'm just not sure what the correct behavior should be. Sorry
> if this pollutes the discussion, I'm completely new to the LKML.
Good find! I suggest we leave that separately though. But as I am also
completely new to the LKML I don't know what best practices here are.
Let's wait for maintainers to chime in :)
>
> I'm not sure what else I could try, I'll keep your V2 on my system and
> let you know if any issues pop up.
Great, thank you!
>
> Regards,
> Kate
>
> On 7/14/26 2:54 AM, Erik Håkansson wrote:
>> Hi,
>>
>> Thank you very much for your help.
>>
>> Unfortunately I don't have a Windows system easily accessible, so I
>> can't check my firmware version in Options+.
>> fwupdmgr says:
>>> Current version: MPR05.03_B0020
>> so perhaps that means 5.03.20.
>>
>> Let me know if anything pops up if you dig some more,
>> Thanks,
>> Erik
^ permalink raw reply
* [PATCH] HID: uclogic: Clear stale pen_input pointer on partial input registration
From: Doruk Tan Ozturk @ 2026-07-14 14:25 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel, stable
uclogic_input_configured() caches the pen input_dev in
drvdata->pen_input before input_register_device() is called for it.
The in-range timeout handler uclogic_inrange_timeout() later
dereferences this pointer, guarded only by a NULL check.
If hidinput_connect() fails while bringing up the HID inputs (e.g. an
input_register_device() failure, or an input left unpopulated), it
unwinds via hidinput_disconnect() and frees the input_dev, but the
driver's cached drvdata->pen_input is not cleared and is left dangling.
Because this driver installs a ->raw_event callback and the hidraw
interface is claimed, hid_connect() still returns success even though
HID input was not claimed, so hid_hw_start() and probe() succeed. The
device stays live, and a subsequent in-range pen report re-arms
inrange_timer via uclogic_raw_event_pen(). When the timer fires,
uclogic_inrange_timeout() dereferences the freed input_dev: the NULL
check does not help because the pointer is dangling, not NULL. This is
a use-after-free distinct from the timer-teardown case.
Clear drvdata->pen_input after hid_hw_start() when HID input was not
claimed, so the cached pointer never outlives the input_dev and the
existing NULL check in the timeout handler becomes effective.
Found by 0sec (https://0sec.ai).
Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/hid/hid-uclogic-core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..396d46b0b88e 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -262,6 +262,19 @@ static int uclogic_probe(struct hid_device *hdev,
goto failure;
}
+ /*
+ * hid_hw_start() -> hid_connect() returns success even when
+ * hidinput_connect() failed, because this driver provides a
+ * ->raw_event callback and the hidraw interface was claimed. In that
+ * case the input_dev that ->input_configured() cached in
+ * drvdata->pen_input has already been freed by hidinput_connect()'s
+ * error unwinding, leaving a dangling pointer that the in-range timer
+ * would dereference. Drop it so uclogic_inrange_timeout()'s NULL
+ * check takes effect.
+ */
+ if (!(hdev->claimed & HID_CLAIMED_INPUT))
+ drvdata->pen_input = NULL;
+
return 0;
failure:
/* Assume "remove" might not be called if "probe" failed */
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 4/4] HID: cp2112: fix out-of-bounds read in cp2112_raw_event
From: sashiko-bot @ 2026-07-14 14:04 UTC (permalink / raw)
To: Jiale Yao; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260714131611.269929-4-yaojiale02@163.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cancel_delayed_work_sync() on the probe error path can lead to Use-After-Free of devm-allocated dev structure.
- [High] Race condition in cp2112_remove() due to premature and conditional cancellation of the delayed work.
- [High] Lockless updates to dev->read_data and dev->read_length lack required memory barriers when signaled via atomic flags.
- [High] A TOCTOU race condition on dev->read_length in cp2112_read() can bypass length checks and return uninitialized kernel memory.
- [High] Using an interruptible wait in cp2112_wait() allows signals to permanently desynchronize the hardware response state.
--
--- Patch [4]: [PATCH 4/4] HID: cp2112: fix out-of-bounds read in cp2112_raw_event ---
commit 1ea70eb400858ee9a4d1aa2528079b6370e4b0a8
Author: Jiale Yao <yaojiale02@163.com>
HID: cp2112: fix out-of-bounds read in cp2112_raw_event
The TRANSFER_STATUS_RESPONSE path dereferences xfer->length at
offset 6, requiring sizeof(*xfer) bytes. The DATA_READ_RESPONSE
path uses the device-supplied read_length to drive memcpy()
without clamping it to the remaining report size. Add both
checks, and clamp read_length to size - 3.
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 04379db93571b..99e31724a4211 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
[ ... ]
> @@ -1463,12 +1468,18 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
> atomic_set(&dev->xfer_avail, 1);
> break;
> case CP2112_DATA_READ_RESPONSE:
> + if (size < 4) {
> + hid_err(hdev, "short data read response(%d < 4)\n", size);
> + return 0;
> + }
> hid_dbg(hdev, "read response: %02x %02x\n", data[1], data[2]);
>
> dev->read_length = data[2];
> if (dev->read_length > sizeof(dev->read_data))
> dev->read_length = sizeof(dev->read_data);
>
> + if (dev->read_length > size - 3)
> + dev->read_length = size - 3;
> memcpy(dev->read_data, &data[3], dev->read_length);
> atomic_set(&dev->read_avail, 1);
[Severity: High]
This is a pre-existing issue, but does this lockless update to dev->read_data
and dev->read_length lack required memory barriers?
In cp2112_raw_event(), dev->read_data is updated and then read_avail is
signaled locklessly:
memcpy(dev->read_data, &data[3], dev->read_length);
atomic_set(&dev->read_avail, 1);
However, the consumer in cp2112_wait() evaluates atomic_read() locklessly,
and cp2112_read() reads the payload without an acquire barrier:
cp2112_wait():
ret = wait_event_interruptible_timeout(dev->wait,
atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));
cp2112_read():
memcpy(data, dev->read_data, size);
On weakly ordered architectures, could the CPU reorder the producer's data
write after the flag set, or the consumer's data read before the flag check,
leading to stale memory reads and I2C data corruption?
[Severity: High]
This is a pre-existing issue, but does the error path in cp2112_probe()
leave the delayed work running?
drivers/hid/hid-cp2112.c:cp2112_probe() {
...
err_gpiochip_remove:
gpiochip_remove(&dev->gc);
err_free_i2c:
...
}
If gpiochip_add_data() succeeds, the GPIO chip is exposed and its IRQ can be
immediately requested, scheduling the gpio_poll_worker.
If a subsequent initialization step fails, the probe function jumps to
err_gpiochip_remove and removes the gpiochip, but fails to cancel the
delayed work before the dev structure is freed.
Could this lead to a Use-After-Free of the dev structure?
[Severity: High]
This is a pre-existing issue, but is there a race condition in
cp2112_remove() due to premature cancellation of the delayed work?
drivers/hid/hid-cp2112.c:cp2112_remove() {
...
if (dev->gpio_poll) {
dev->gpio_poll = false;
cancel_delayed_work_sync(&dev->gpio_poll_worker);
}
gpiochip_remove(&dev->gc);
...
}
Because the GPIO chip is still registered when the cancellation is evaluated,
a concurrent IRQ request can execute cp2112_gpio_irq_startup(), setting
dev->gpio_poll = true and scheduling the work just after the cancellation
check but before the chip is removed.
Once remove() completes and dev is freed, will the scheduled work cause a
Use-After-Free?
[Severity: High]
This is a pre-existing issue, but is there a TOCTOU race on
dev->read_length in cp2112_read()?
drivers/hid/hid-cp2112.c:cp2112_read() {
...
if (size > dev->read_length)
size = dev->read_length;
memcpy(data, dev->read_data, size);
return dev->read_length;
}
If a spurious or delayed USB CP2112_DATA_READ_RESPONSE packet is processed
concurrently by cp2112_raw_event(), dev->read_length can increase between
the bounds check and the return value.
Since cp2112_read() reads dev->read_length locklessly twice, could it copy
fewer bytes than it returns? This would cause the caller cp2112_i2c_xfer()
to advance its buffer by the larger returned value and leak uninitialized
heap memory to the userspace I2C client.
[Severity: High]
This is a pre-existing issue, but does using an interruptible wait in
cp2112_wait() allow signals to permanently desynchronize the hardware
response state?
drivers/hid/hid-cp2112.c:cp2112_wait() {
...
ret = wait_event_interruptible_timeout(dev->wait,
atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));
if (-ERESTARTSYS == ret)
return ret;
...
}
If a user process executing an I2C transaction receives a signal,
wait_event_interruptible_timeout() aborts with -ERESTARTSYS. The driver
exits, but the hardware will eventually deliver the response for the
aborted transaction.
Because the hardware lacks transaction IDs, will this late response falsely
satisfy the cp2112_wait() of the next I2C transaction, returning garbage
data or false success?
Should the driver use wait_event_timeout() for short hardware
synchronization instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131611.269929-1-yaojiale02@163.com?part=4
^ permalink raw reply
* Re: [PATCH v9 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys
From: Svyatoslav Ryhel @ 2026-07-14 14:03 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Michał Mirosław, Ion Agorria,
Sebastian Reichel, Pavel Machek, Lee Jones, Conor Dooley,
Krzysztof Kozlowski, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <CAPVz0n1o+2NRQrLfbCMT0JdeFhhD4PS4UzSt3L6-QQ2ywHL+mA@mail.gmail.com>
пт, 3 лип. 2026 р. о 11:53 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> чт, 25 черв. 2026 р. о 11:16 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> >
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Add support for multimedia top button row of ASUS Transformer's Mobile
> > Dock keyboard. Driver is made that function keys (F1-F12) are used by
> > default which suits average Linux use better and with pressing
> > ScreenLock + AltGr function keys layout is switched to multimedia keys.
> > Only Dock keyboard input events are tracked for AltGr pressing.
> >
> > Co-developed-by: Ion Agorria <ion@agorria.com>
> > Signed-off-by: Ion Agorria <ion@agorria.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/input/keyboard/Kconfig | 10 +
> > drivers/input/keyboard/Makefile | 1 +
> > .../input/keyboard/asus-transformer-ec-keys.c | 314 ++++++++++++++++++
> > 3 files changed, 325 insertions(+)
> > create mode 100644 drivers/input/keyboard/asus-transformer-ec-keys.c
> >
> > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> > index 9d1019ba0245..913cb4900565 100644
> > --- a/drivers/input/keyboard/Kconfig
> > +++ b/drivers/input/keyboard/Kconfig
> > @@ -89,6 +89,16 @@ config KEYBOARD_APPLESPI
> > To compile this driver as a module, choose M here: the
> > module will be called applespi.
> >
> > +config KEYBOARD_ASUS_TRANSFORMER_EC
> > + tristate "Asus Transformer's Mobile Dock multimedia keys"
> > + depends on MFD_ASUS_TRANSFORMER_EC
> > + help
> > + Say Y here if you want to use multimedia keys present on Asus
> > + Transformer's Mobile Dock.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called asus-transformer-ec-keys.
> > +
> > config KEYBOARD_ATARI
> > tristate "Atari keyboard"
> > depends on ATARI
> > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> > index 60bb7baf802f..0d81096887ad 100644
> > --- a/drivers/input/keyboard/Makefile
> > +++ b/drivers/input/keyboard/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_ADP5585) += adp5585-keys.o
> > obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
> > obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
> > obj-$(CONFIG_KEYBOARD_APPLESPI) += applespi.o
> > +obj-$(CONFIG_KEYBOARD_ASUS_TRANSFORMER_EC) += asus-transformer-ec-keys.o
> > obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
> > obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
> > obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
> > diff --git a/drivers/input/keyboard/asus-transformer-ec-keys.c b/drivers/input/keyboard/asus-transformer-ec-keys.c
> > new file mode 100644
> > index 000000000000..53aff3ce7146
> > --- /dev/null
> > +++ b/drivers/input/keyboard/asus-transformer-ec-keys.c
> > @@ -0,0 +1,314 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <linux/array_size.h>
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/mfd/asus-transformer-ec.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#define ASUSEC_EXT_KEY_CODES 0x20
> > +
> > +struct asus_ec_keys_data {
> > + struct notifier_block nb;
> > + struct asusec_core *ec;
> > + struct input_dev *xidev;
> > + struct input_handler input_handler;
> > + unsigned short keymap[ASUSEC_EXT_KEY_CODES * 2];
> > + const char *kbc_phys;
> > + bool special_key_pressed;
> > + bool special_key_mode;
> > +};
> > +
> > +static void asus_ec_input_event(struct input_handle *handle,
> > + unsigned int event_type,
> > + unsigned int event_code, int value)
> > +{
> > + struct asus_ec_keys_data *priv = handle->handler->private;
> > +
> > + /* Store special key state */
> > + if (event_type == EV_KEY && event_code == KEY_RIGHTALT)
> > + priv->special_key_pressed = !!value;
> > +}
> > +
> > +static int asus_ec_input_connect(struct input_handler *handler,
> > + struct input_dev *dev,
> > + const struct input_device_id *id)
> > +{
> > + struct asus_ec_keys_data *priv = handler->private;
> > + struct input_handle *handle;
> > + int error;
> > +
> > + if (!dev->phys || !strstr(dev->phys, priv->kbc_phys))
> > + return -ENODEV;
> > +
>
> Hello Dmitry!
>
> Would this approach be acceptable? Handler links strictly to asus-ec keyboard.
>
Hello Dmitry! May you please take a look? ATM only ACK on input patches left.
^ permalink raw reply
* Re: [PATCH 3/4] HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event
From: sashiko-bot @ 2026-07-14 13:52 UTC (permalink / raw)
To: Jiale Yao; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260714131611.269929-3-yaojiale02@163.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Stale mcp->rxbuf pointer leads to Use-After-Free or Stack Buffer Overflow when receiving delayed MCP2221_I2C_GET_DATA input reports.
- [High] An out-of-bounds read occurs in mcp2221_raw_event due to uninitialized or shared mcp->gp_idx state.
- [High] Shared transfer state variables such as mcp->rxbuf_idx are modified concurrently by process context and the HID raw event handler without locking.
- [High] The `retries` counter in `mcp_init_work` is declared as a `static int`, leading to data races and logic bugs when multiple mcp2221 devices are probed or a single device is replugged.
--
--- Patch [3]: [PATCH 3/4] HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event ---
commit 8bc8521d47c1c74b70fcc5afb281169b37669d79
Author: Jiale Yao <yaojiale02@163.com>
HID: mcp2221: fix out-of-bounds read in mcp2221_raw_event
This commit adds bounds checking to the mcp2221_raw_event handler to
prevent out-of-bounds reads when processing HID reports. It introduces a
top-level size check and specific size constraints for different I2C and
GPIO operations.
> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> index e4ddd8e9293b6..0423b5affe8f6 100644
> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
[ ... ]
> @@ -918,7 +931,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
> }
> if (data[2] == MCP2221_I2C_READ_COMPL ||
> data[2] == MCP2221_I2C_READ_PARTIAL) {
> - if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) {
> + if (!mcp->rxbuf || mcp->rxbuf_idx < 0 ||
> + data[3] > 60 || size < 4 + data[3]) {
> mcp->status = -EINVAL;
> break;
> }
[Severity: Critical]
This is a pre-existing issue, but does this code write to a dangling
stack pointer if a delayed input report arrives?
Looking at mcp_i2c_smbus_read(), mcp->rxbuf is pointed to a local stack
buffer. If the I2C request times out after 5 retries, mcp_i2c_smbus_read()
returns without clearing mcp->rxbuf:
mcp_i2c_smbus_read() {
...
retries++;
if (retries == 5)
return -ETIMEDOUT;
...
}
Could a malicious USB device send a delayed MCP2221_I2C_GET_DATA report
after the timeout, causing mcp2221_raw_event() to locklessly copy up to
60 bytes into the now-freed stack frame?
[Severity: High]
This is also a pre-existing issue, but is it safe to modify mcp->rxbuf_idx
here without a lock?
The I2C process thread holds mcp->lock while initializing mcp->rxbuf_idx
in mcp_i2c_smbus_read():
mcp_i2c_smbus_read() {
...
mcp->rxbuf_idx = 0;
...
}
However, mcp2221_raw_event() runs asynchronously in softirq context. Can a
spurious input report race with the process-context operations, corrupting
the buffer bounds or indices?
[ ... ]
> @@ -958,6 +974,8 @@ static int mcp2221_raw_event(struct hid_device *hdev,
> break;
>
> case MCP2221_GPIO_SET:
> + if (size < sizeof(struct mcp_set_gpio))
> + return 1;
> switch (data[1]) {
> case MCP2221_SUCCESS:
> if ((data[mcp->gp_idx] == MCP2221_ALT_F_NOT_GPIOV) ||
[Severity: High]
This is a pre-existing issue, but does this code perform an out-of-bounds
access if a device sends an unsolicited report before any GPIO operations
occur?
If mcp->gp_idx is still 0, evaluating data[mcp->gp_idx - 1] in the
MCP2221_GPIO_SET handler will read from data[-1].
Similarly, if mcp->gp_idx is legitimately set to a high value like 17,
could an unsolicited MCP2221_GPIO_GET report with a valid size of 10
access data[18], reading past the bounds of the short packet? The newly
added check for the overall packet size does not prevent this since it
doesn't validate the independently modified mcp->gp_idx.
[ ... ]
[Severity: High]
This is a pre-existing issue outside the modified hunks, but is the
retries counter in mcp_init_work() susceptible to data races across
multiple devices?
mcp_init_work() defines retries as a static variable:
mcp_init_work() {
static int retries = 5;
...
if (!retries--)
return;
...
schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(100));
}
If a device fails IIO initialization multiple times, the static counter
drops to -1. Once negative, !retries-- evaluates to false, causing an
infinite rescheduling loop for any subsequent devices or replugs that
fail. Should this retry state be tracked per-device in the mcp2221
struct instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131611.269929-1-yaojiale02@163.com?part=3
^ permalink raw reply
* Re: [PATCH] HID: input: allocate input_dev name/phys/uniq using hid device devres
From: chlgustn3171 @ 2026-07-14 13:39 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-input, dmitry.torokhov, Jiri Kosina, Benjamin Tissoires,
linux-kernel
In-Reply-To: <CAO2ma08Pap6PZKUV7iEpvu11zaKrzMK-cgBLBYKJEEfGH+0=1g@mail.gmail.com>
Adding maintainers and mailing list to CC. Sorry for the omission.
On Tue, Jul 14, 2026 at 10:33 PM chlgustn3171 <chlgustn3171@gmail.com> wrote:
>
> On Tue, Jul 14, 2026 at 9:18 PM sashiko-bot wrote:
> > [Severity: High]
> > If a userspace process holds an open file descriptor for the evdev node
> > (/dev/input/eventX), the input_dev will outlive the parent hid->dev.
> > When the HID device is unbound, devres will free these strings.
> >
> > Can this cause a use-after-free when userspace later queries the
> > name, phys, or uniq strings via ioctl (e.g., EVIOCGNAME)?
>
> Thanks for the review.
>
> Using &hid->dev here is intentional.
> Using the input device for devm-managed input_dev strings is
> problematic; see CVE-2023-53454, where the allocation was moved to the
> HID device because the input device unregister path may still use the
> name for uevents.
>
> > [Severity: Low]
> > If devm_kstrdup() fails for phys or uniq, the code jumps here to the
> > error path. The earlier successful devm_kstrdup() allocations remain
> > tied to &hid->dev.
> >
> > If the caller continues the device probe successfully after this failure,
> > will these strings be leaked for the lifetime of the HID device?
>
> The name/phys/uniq copies are attached to &hid->dev, so the clean-up
> is handled by HID device devres.
> However, if it is better to explicitly unwind, I can add that in v2.
>
> Thanks,
> Hyeonsu
^ permalink raw reply
* Re: [PATCH] HID: input: allocate input_dev name/phys/uniq using hid device devres
From: chlgustn3171 @ 2026-07-14 13:33 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260714121819.5FCC91F00A3A@smtp.kernel.org>
On Tue, Jul 14, 2026 at 9:18 PM sashiko-bot wrote:
> [Severity: High]
> If a userspace process holds an open file descriptor for the evdev node
> (/dev/input/eventX), the input_dev will outlive the parent hid->dev.
> When the HID device is unbound, devres will free these strings.
>
> Can this cause a use-after-free when userspace later queries the
> name, phys, or uniq strings via ioctl (e.g., EVIOCGNAME)?
Thanks for the review.
Using &hid->dev here is intentional.
Using the input device for devm-managed input_dev strings is
problematic; see CVE-2023-53454, where the allocation was moved to the
HID device because the input device unregister path may still use the
name for uevents.
> [Severity: Low]
> If devm_kstrdup() fails for phys or uniq, the code jumps here to the
> error path. The earlier successful devm_kstrdup() allocations remain
> tied to &hid->dev.
>
> If the caller continues the device probe successfully after this failure,
> will these strings be leaked for the lifetime of the HID device?
The name/phys/uniq copies are attached to &hid->dev, so the clean-up
is handled by HID device devres.
However, if it is better to explicitly unwind, I can add that in v2.
Thanks,
Hyeonsu
^ permalink raw reply
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