* Re: [PATCH v6 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Dmitry Torokhov @ 2026-06-26 20:20 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
linux-input, devicetree, linux-kernel
In-Reply-To: <CAPVz0n0Ja0eKrpirtMxK8bkTV0NY-UoKU0c09qLEF-xMWmOzKQ@mail.gmail.com>
On Fri, Jun 26, 2026 at 10:45:20AM +0300, Svyatoslav Ryhel wrote:
> пт, 26 черв. 2026 р. о 02:45 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
> >
> > Hi Svyatoslav,
> >
> > On Wed, Jun 17, 2026 at 10:05:27AM +0300, Svyatoslav Ryhel wrote:
> > > From: Linus Walleij <linusw@kernel.org>
> > >
> > > The ISA1200 is a haptic feedback unit from Imagis Technology using two
> > > motors for haptic feedback in mobile phones. Used in many mobile devices
> > > c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> > > GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
> > >
> > > The exact datasheet for the ISA1200 is not available; all data was modeled
> > > based on available downstream kernel sources for various devices and
> > > fragments of information scattered across the internet.
> > >
> > > Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> > > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >
> >
> > Sashiko convinced me that using mutex_trylock() in the playback work
> > handler will result in dropping requests and that we are safe not taking
> > the lock there at all. Can you please try the following modification?
> >
> > diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> > index c61adc4b605c..fb7f68fa0a2b 100644
> > --- a/drivers/input/misc/isa1200.c
> > +++ b/drivers/input/misc/isa1200.c
> > @@ -253,15 +253,12 @@ static void isa1200_stop(struct isa1200 *isa)
> > static void isa1200_play_work(struct work_struct *work)
> > {
> > struct isa1200 *isa = container_of(work, struct isa1200, play_work);
> > - struct input_dev *input = isa->input;
> > -
> > - scoped_guard(mutex_try, &input->mutex) {
> > - if (!isa->suspended) {
> > - if (isa->level)
> > - isa1200_start(isa);
> > - else
> > - isa1200_stop(isa);
> > - }
> > +
> > + if (!READ_ONCE(isa->suspended)) {
> > + if (isa->level)
> > + isa1200_start(isa);
> > + else
> > + isa1200_stop(isa);
> > }
> > }
> >
> >
> > If this works no need to resend, I'll fold on my side.
> >
>
> It does work fine. Thank you.
Excelled, thank you. Applied both patches.
--
Dmitry
^ permalink raw reply
* [regression] synaptics-rmi4: Failed to read the Control Register Descriptor: EIO
From: David Heidelberg @ 2026-06-26 21:09 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: Linux Next Mailing List
With next-20260626 without any additional patches, the synaptics fails on
OnePlus 6T with:
```
[ 16.620292] rmi4_i2c 12-0020: registering I2C-connected sensor
[ 16.857617] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics,
product: S3706B, fw id: 2852315
[ 16.867324] rmi4_f12 rmi4-00.fn12: Failed to read the Control Register
Descriptor: -5
[ 16.867338] rmi4_f12 rmi4-00.fn12: probe with driver rmi4_f12 failed with
error -5
```
I assume it's due to refactoring. My best guess it's signed/unsigned 32/64bit
type changes, but since I haven't figured it out within reasonable timeframe.
David
^ permalink raw reply
* Re: [regression] synaptics-rmi4: Failed to read the Control Register Descriptor: EIO
From: Dmitry Torokhov @ 2026-06-26 21:18 UTC (permalink / raw)
To: David Heidelberg; +Cc: linux-input, Linux Next Mailing List
In-Reply-To: <de6d34b7-619c-46bd-a6e3-a3917b318571@ixit.cz>
Hi David,
On Fri, Jun 26, 2026 at 11:09:54PM +0200, David Heidelberg wrote:
> With next-20260626 without any additional patches, the synaptics fails on
> OnePlus 6T with:
>
> ```
> [ 16.620292] rmi4_i2c 12-0020: registering I2C-connected sensor
> [ 16.857617] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer:
> Synaptics, product: S3706B, fw id: 2852315
> [ 16.867324] rmi4_f12 rmi4-00.fn12: Failed to read the Control Register
> Descriptor: -5
> [ 16.867338] rmi4_f12 rmi4-00.fn12: probe with driver rmi4_f12 failed with
> error -5
> ```
Yes, at least a98518e72439 "Input: rmi4 - fix register descriptor address
calculation" is wrong and needs to be reverted.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 0/2] Input: ilitek_ts: add stylus support for 0x0c reports
From: Kristian Mide @ 2026-06-26 21:42 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Kristian Mide
Hi,
I bought a CHUWI Hi10 Max and was annoyed to find that the built-in
stylus was not usable with the upstream ilitek_ts driver.
This small series adds support for the pen report format emitted by the
device, exposing the stylus as a separate input device with hover,
pressure, and side-button support. The pen device is created lazily on
first pen report so touchscreen-only hardware does not advertise stylus
capabilities up front.
The packet format was reverse engineered on this device. In particular,
report ID 0x0c appears to carry pen events, pressure comes from
buf[6..7] shifted right by one, and the side buttons map to two bits in
buf[1].
The second patch is unrelated to the pen support itself, but keeps the
existing wrong-report logging from flooding dmesg when unexpected packets
repeat quickly.
On the tested CHUWI Hi10 Max this makes the pen usable in GNOME/Krita,
including pressure and side buttons.
Thanks,
Kristian
Kristian Mide (2):
Input: ilitek_ts: add stylus input support
Input: ilitek_ts: ratelimit unexpected report logging
drivers/input/touchscreen/ilitek_ts_i2c.c | 134 +++++++++++++++++++++-
1 file changed, 133 insertions(+), 1 deletion(-)
--
2.54.0
^ permalink raw reply
* [PATCH 1/2] Input: ilitek_ts: add stylus input support
From: Kristian Mide @ 2026-06-26 21:42 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Kristian Mide
In-Reply-To: <20260626214248.5563-1-kristian@mide.dk>
Add a separate stylus input device for report ID 0x0c packets,
with pressure, hover, and side-button support.
The pen device is created lazily on first pen report so
touchscreen-only hardware does not expose stylus capabilities up
front.
The packet format is reverse engineered from a tested CHUWI
Hi10 Max. Pressure is reported from buf[6..7] shifted right by
one, matching the observed 1024 pressure levels on the tested
device.
Pen coordinates are reported through touchscreen_report_pos()
so the same axis inversion and swapping properties used by the
touch path apply to the stylus as well.
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 130 ++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 3de0fbf8d..f0721af02 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -18,6 +18,7 @@
#include <linux/gpio/consumer.h>
#include <linux/errno.h>
#include <linux/acpi.h>
+#include <linux/device/devres.h>
#include <linux/input/touchscreen.h>
#include <linux/unaligned.h>
@@ -37,6 +38,18 @@
#define ILITEK_TP_CMD_GET_IC_MODE 0xC0
#define ILITEK_TP_I2C_REPORT_ID 0x48
+/* Reverse engineered stylus report on a tested CHUWI Hi10 Max device. */
+#define ILITEK_PEN_I2C_REPORT_ID 0x0C
+#define ILITEK_PEN_PRESSURE_MAX 1023
+#define ILITEK_PEN_DISTANCE_MAX 2
+/* Userspace expects tablet axis resolution; with INPUT_PROP_DIRECT this is */
+/* mostly descriptive and does not materially affect event coordinates. */
+#define ILITEK_PEN_RESOLUTION 68
+
+#define ILITEK_PEN_FLAG_TOUCH 0x01
+#define ILITEK_PEN_FLAG_STYLUS2 0x02
+#define ILITEK_PEN_FLAG_STYLUS 0x08
+#define ILITEK_PEN_FLAG_PROX 0x10
#define REPORT_COUNT_ADDRESS 61
#define ILITEK_SUPPORT_MAX_POINT 40
@@ -50,6 +63,7 @@ struct ilitek_ts_data {
struct i2c_client *client;
struct gpio_desc *reset_gpio;
struct input_dev *input_dev;
+ struct input_dev *pen_input_dev;
struct touchscreen_properties prop;
const struct ilitek_protocol_map *ptl_cb_func;
@@ -89,6 +103,9 @@ enum ilitek_cmds {
MAX_CMD_CNT
};
+static int ilitek_pen_input_dev_init(struct device *dev,
+ struct ilitek_ts_data *ts);
+
/* ILITEK I2C R/W APIs */
static int ilitek_i2c_write_and_read(struct ilitek_ts_data *ts,
u8 *cmd, int write_len, int delay,
@@ -146,6 +163,62 @@ static void ilitek_touch_down(struct ilitek_ts_data *ts, unsigned int id,
touchscreen_report_pos(input, &ts->prop, x, y, true);
}
+static void ilitek_unregister_pen_input(void *data)
+{
+ struct ilitek_ts_data *ts = data;
+
+ input_unregister_device(ts->pen_input_dev);
+ ts->pen_input_dev = NULL;
+}
+
+/*
+ * buf[1] carries prox/touch/side-button state and buf[6..7] carries
+ * pressure. A right shift by one matches the observed 1024 pressure levels.
+ */
+static int ilitek_process_pen_report(struct ilitek_ts_data *ts, u8 *buf)
+{
+ struct device *dev = &ts->client->dev;
+ struct input_dev *input = ts->pen_input_dev;
+ unsigned int x, y, z, distance;
+ bool prox, touch, stylus, stylus2;
+ int error;
+
+ if (!input) {
+ error = ilitek_pen_input_dev_init(dev, ts);
+ if (error) {
+ dev_err_ratelimited(dev,
+ "failed to register pen input device: %d\n",
+ error);
+ return 0;
+ }
+ input = ts->pen_input_dev;
+ }
+
+ x = get_unaligned_le16(buf + 2);
+ y = get_unaligned_le16(buf + 4);
+ z = get_unaligned_le16(buf + 6) >> 1;
+ prox = !!(buf[1] & ILITEK_PEN_FLAG_PROX);
+ touch = !!(buf[1] & ILITEK_PEN_FLAG_TOUCH);
+ stylus = !!(buf[1] & ILITEK_PEN_FLAG_STYLUS);
+ stylus2 = !!(buf[1] & ILITEK_PEN_FLAG_STYLUS2);
+ distance = prox ? (touch ? 0 : 1) : ILITEK_PEN_DISTANCE_MAX;
+ if (!touch)
+ z = 0;
+ else if (z > ILITEK_PEN_PRESSURE_MAX)
+ z = ILITEK_PEN_PRESSURE_MAX;
+
+ input_report_key(input, BTN_TOOL_PEN, prox || touch);
+ input_report_key(input, BTN_TOUCH, touch);
+ input_report_key(input, BTN_STYLUS, stylus);
+ input_report_key(input, BTN_STYLUS2, stylus2);
+ touchscreen_report_pos(input, &ts->prop, x, y, false);
+ input_report_abs(input, ABS_PRESSURE, z);
+ input_report_abs(input, ABS_DISTANCE, distance);
+ input_sync(input);
+
+ return 0;
+}
+
static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
{
int error = 0;
@@ -164,6 +237,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
return error;
}
+ if (buf[0] == ILITEK_PEN_I2C_REPORT_ID)
+ return ilitek_process_pen_report(ts, buf);
+
if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
return -EINVAL;
@@ -459,6 +535,60 @@ static int ilitek_read_tp_info(struct ilitek_ts_data *ts, bool boot)
return 0;
}
+static int ilitek_pen_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
+{
+ struct input_dev *pen_input;
+ int error;
+
+ if (ts->pen_input_dev)
+ return 0;
+
+ /* No explicit pen capability probe is known; create on first pen report. */
+
+ pen_input = input_allocate_device();
+ if (!pen_input)
+ return -ENOMEM;
+
+ ts->pen_input_dev = pen_input;
+ pen_input->dev.parent = dev;
+ pen_input->name = "ilitek_ts_pen";
+ pen_input->id.bustype = BUS_I2C;
+
+ __set_bit(INPUT_PROP_DIRECT, pen_input->propbit);
+ __set_bit(EV_KEY, pen_input->evbit);
+ __set_bit(EV_ABS, pen_input->evbit);
+ __set_bit(BTN_TOUCH, pen_input->keybit);
+ __set_bit(BTN_TOOL_PEN, pen_input->keybit);
+ __set_bit(BTN_STYLUS, pen_input->keybit);
+ __set_bit(BTN_STYLUS2, pen_input->keybit);
+
+ input_set_abs_params(pen_input, ABS_X,
+ ts->screen_min_x, ts->screen_max_x, 0, 0);
+ input_set_abs_params(pen_input, ABS_Y,
+ ts->screen_min_y, ts->screen_max_y, 0, 0);
+ input_set_abs_params(pen_input, ABS_PRESSURE, 0,
+ ILITEK_PEN_PRESSURE_MAX, 0, 0);
+ input_set_abs_params(pen_input, ABS_DISTANCE, 0,
+ ILITEK_PEN_DISTANCE_MAX, 0, 0);
+ input_abs_set_res(pen_input, ABS_X, ILITEK_PEN_RESOLUTION);
+ input_abs_set_res(pen_input, ABS_Y, ILITEK_PEN_RESOLUTION);
+
+ error = input_register_device(pen_input);
+ if (error)
+ goto err_free_pen_input;
+
+ error = devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts);
+ if (error)
+ return error;
+
+ return 0;
+
+err_free_pen_input:
+ ts->pen_input_dev = NULL;
+ input_free_device(pen_input);
+ return error;
+}
+
static int ilitek_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
{
int error;
--
2.54.0
^ permalink raw reply related
* [PATCH 2/2] Input: ilitek_ts: ratelimit unexpected report logging
From: Kristian Mide @ 2026-06-26 21:42 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Kristian Mide
In-Reply-To: <20260626214248.5563-1-kristian@mide.dk>
Unexpected report IDs can repeat quickly when the controller
emits unknown packets.
Use dev_err_ratelimited() and include the first 16 bytes of the
raw packet to keep dmesg usable while preserving enough context
for debugging.
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index f0721af02..8886bd39b 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -241,7 +241,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
return ilitek_process_pen_report(ts, buf);
if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
- dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
+ dev_err_ratelimited(dev,
+ "get touch info failed. Wrong id: 0x%02X raw[0:16]=%*ph\n",
+ buf[0], 16, buf);
return -EINVAL;
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 2/2] Input: ilitek_ts: ratelimit unexpected report logging
From: sashiko-bot @ 2026-06-26 21:56 UTC (permalink / raw)
To: Kristian Mide; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260626214248.5563-3-kristian@mide.dk>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The I2C read/write wrapper does not properly check for short transfers, and this patch leaks the resulting uninitialized stack memory to dmesg.
Pre-existing issues:
- [High] The patch fails to prevent dmesg flooding and causes the interrupt handler to incorrectly signal a spurious interrupt, which can permanently disable the touchscreen.
- [High] Dynamically adding a devres action in a threaded IRQ handler creates an inverted teardown order during device unbind, leading to a memory leak and Use-After-Free.
- [High] If the I2C sleep command fails during suspend, the function returns an error without re-enabling the IRQ, rendering the touchscreen permanently unresponsive.
- [High] The driver passes stack-allocated arrays to i2c_transfer(), violating DMA mapping rules and risking memory corruption.
--
commit ec332d9cd1c7923c04e4eec0684513387a92348d
Author: Kristian Mide <kristian@mide.dk>
Input: ilitek_ts: ratelimit unexpected report logging
Unexpected report IDs can repeat quickly when the controller emits unknown
packets. This commit uses dev_err_ratelimited() and includes the first 16
bytes of the raw packet for context.
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index f0721af02adf2..8886bd39bc0b6 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -241,7 +241,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
[Severity: High]
This is a pre-existing issue, but does ilitek_process_and_report_v6() pass a
stack-allocated array to i2c_transfer()?
drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_process_and_report_v6() {
...
u8 buf[512];
...
error = ilitek_i2c_write_and_read(ts, NULL, 0, 0, buf, 64);
If the underlying I2C host controller driver maps the buffer for DMA, it might
attempt to map a stack address. On modern kernels where VMAP_STACK is enabled,
can this cause an immediate panic because stack memory is not physically
contiguous, or lead to cacheline sharing corruption otherwise?
> return ilitek_process_pen_report(ts, buf);
[Severity: High]
This is a pre-existing issue, but does dynamically adding a devres action
in the threaded IRQ handler create an inverted teardown order?
The IRQ is registered via devm_request_threaded_irq() in probe. Later, the
ISR dynamically calls devm_add_action_or_reset() to register the pen cleanup:
drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_process_pen_report() {
...
if (!input) {
error = ilitek_pen_input_dev_init(dev, ts);
which calls devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts).
Because devm is a LIFO stack, the pen cleanup is placed above the IRQ release
action. During device unbind, devres_release_all() will run the pen cleanup
first, freeing the device, while the IRQ remains active. If an interrupt fires
before the IRQ is finally freed, could the ISR access the freed device and
cause a use-after-free, or attempt to append a new devres action while the
stack is being torn down?
>
> if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
> - dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
> + dev_err_ratelimited(dev,
> + "get touch info failed. Wrong id: 0x%02X raw[0:16]=%*ph\n",
> + buf[0], 16, buf);
[Severity: High]
Does this new logging leak uninitialized kernel stack memory to dmesg?
Looking at ilitek_i2c_write_and_read(), it checks if error < 0, but
i2c_transfer() returns the number of successfully transferred messages:
drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_i2c_write_and_read() {
...
error = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
if (error < 0)
return error;
If a short transfer occurs, this evaluates to false, so the wrapper returns 0
(success) despite the read buffer not being populated. When that happens,
won't this new log message directly print 16 bytes of uninitialized kernel
stack memory to the dmesg buffer?
> return -EINVAL;
[Severity: High]
This is a pre-existing issue, but does returning -EINVAL here cause the
interrupt handler to incorrectly signal a spurious interrupt?
When an unknown packet is received, ilitek_process_and_report_v6() returns
-EINVAL to the ISR, which then handles the error like this:
drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_i2c_isr() {
...
error = ilitek_process_and_report_v6(ts);
if (error < 0) {
dev_err(&ts->client->dev, "[%s] err:%d\n", __func__, error);
return IRQ_NONE;
}
First, the ISR unconditionally logs the error, which defeats the ratelimiting
added in this patch. Second, could repeatedly returning IRQ_NONE when the
hardware interrupt was actually serviced (cleared by the I2C read) cause the
kernel's spurious interrupt detection to eventually disable the IRQ line
permanently?
> }
>
[ ... ]
[Severity: High]
This is a pre-existing issue, but does ilitek_suspend() return an error
without re-enabling the previously disabled IRQ?
drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_suspend() {
...
disable_irq(client->irq);
if (!device_may_wakeup(dev)) {
error = api_protocol_set_cmd(ts, SET_IC_SLEEP, NULL, NULL);
if (error)
return error;
}
If the I2C sleep command fails during suspend, the function returns an error
while the device IRQ is disabled. The PM core will abort the suspend
transition and resume previously suspended devices, but it will not call the
resume callback for the device that returned the error. Could this render the
touchscreen permanently unresponsive until reboot or module reload?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626214248.5563-1-kristian@mide.dk?part=2
^ permalink raw reply
* Re: [PATCH 1/2] Input: ilitek_ts: add stylus input support
From: sashiko-bot @ 2026-06-26 21:56 UTC (permalink / raw)
To: Kristian Mide; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260626214248.5563-2-kristian@mide.dk>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Lazy initialization of the pen input device from within the IRQ handler causes a devres LIFO ordering inversion, leading to a memory leak and Use-After-Free during device unbind.
- [High] The pen input device axis limits are not synced with Device Tree properties, leading to mismatched axis boundaries and massive coordinate underflow values.
- [Low] Direct bit manipulation is used instead of the preferred `input_set_capability()` helper.
Pre-existing issues:
- [High] I2C transfers are performed using buffers allocated on the stack, which is unsafe for DMA.
--
commit 0912736a7a818d72cc6868aba6ad50c6f06afd78
Author: Kristian Mide <kristian@mide.dk>
Input: ilitek_ts: add stylus input support
Add a separate stylus input device for report ID 0x0c packets,
with pressure, hover, and side-button support. The pen device is
created lazily on first pen report so touchscreen-only hardware does
not expose stylus capabilities up front.
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index 3de0fbf8da386..f0721af02adf2 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
[ ... ]
> @@ -164,6 +237,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
> return error;
> }
>
> + if (buf[0] == ILITEK_PEN_I2C_REPORT_ID)
> + return ilitek_process_pen_report(ts, buf);
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but since buf is passed here, is it safe to allocate buf as a
512-byte array on the stack in ilitek_process_and_report_v6() and pass it to
I2C transfer functions?
When CONFIG_VMAP_STACK is enabled, stack addresses cannot be safely mapped
for DMA. If the underlying I2C host controller uses DMA, could attempting
to map this stack buffer lead to memory corruption or kernel panics?
> +
> if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
> dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
> return -EINVAL;
[ ... ]
> @@ -459,6 +535,60 @@ static int ilitek_read_tp_info(struct ilitek_ts_data *ts, bool boot)
> return 0;
> }
>
> +static int ilitek_pen_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
> +{
> + struct input_dev *pen_input;
> + int error;
> +
> + if (ts->pen_input_dev)
> + return 0;
> +
> + /* No explicit pen capability probe is known; create on first pen report. */
> +
> + pen_input = input_allocate_device();
> + if (!pen_input)
> + return -ENOMEM;
> +
> + ts->pen_input_dev = pen_input;
> + pen_input->dev.parent = dev;
> + pen_input->name = "ilitek_ts_pen";
> + pen_input->id.bustype = BUS_I2C;
> +
> + __set_bit(INPUT_PROP_DIRECT, pen_input->propbit);
> + __set_bit(EV_KEY, pen_input->evbit);
> + __set_bit(EV_ABS, pen_input->evbit);
> + __set_bit(BTN_TOUCH, pen_input->keybit);
> + __set_bit(BTN_TOOL_PEN, pen_input->keybit);
> + __set_bit(BTN_STYLUS, pen_input->keybit);
> + __set_bit(BTN_STYLUS2, pen_input->keybit);
[Severity: Low]
This isn't a bug, but could we use the input_set_capability() helper here
instead of direct bit manipulation?
> +
> + input_set_abs_params(pen_input, ABS_X,
> + ts->screen_min_x, ts->screen_max_x, 0, 0);
> + input_set_abs_params(pen_input, ABS_Y,
> + ts->screen_min_y, ts->screen_max_y, 0, 0);
[Severity: High]
Should the touchscreen properties (DT properties) be applied to these
absolute parameters?
The touchscreen_report_pos() function called during event processing will
apply inversion and swapping based on Device Tree properties to the
coordinates.
If the limits advertised here are strictly from the raw hardware bounds and
are unaware of those DT properties, could this cause the device to report
out-of-bounds coordinates or result in unsigned underflows when an axis is
inverted?
> + input_set_abs_params(pen_input, ABS_PRESSURE, 0,
> + ILITEK_PEN_PRESSURE_MAX, 0, 0);
> + input_set_abs_params(pen_input, ABS_DISTANCE, 0,
> + ILITEK_PEN_DISTANCE_MAX, 0, 0);
> + input_abs_set_res(pen_input, ABS_X, ILITEK_PEN_RESOLUTION);
> + input_abs_set_res(pen_input, ABS_Y, ILITEK_PEN_RESOLUTION);
> +
> + error = input_register_device(pen_input);
> + if (error)
> + goto err_free_pen_input;
> +
> + error = devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts);
[Severity: High]
Since this function is called lazily from the threaded IRQ handler after
probe has completed, does adding a devm action here break the LIFO teardown
ordering?
During device unbind, devres will release resources in LIFO order. This
action would run first and set ts->pen_input_dev = NULL, but the IRQ
handler would still be active.
If a hardware interrupt fires during unbind after this has been
unregistered, would the IRQ handler observe the NULL pointer, allocate a
new pen device, and add a new devres action that devres_release_all()
will never process, resulting in a memory leak and a potential
use-after-free?
> + if (error)
> + return error;
> +
> + return 0;
> +
> +err_free_pen_input:
> + ts->pen_input_dev = NULL;
> + input_free_device(pen_input);
> + return error;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626214248.5563-1-kristian@mide.dk?part=1
^ permalink raw reply
* Re: [PATCH v6 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Vicki Pfau @ 2026-06-27 1:27 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260625051651.723051F000E9@smtp.kernel.org>
More LLM hallucinations to deal with:
On 6/24/26 10:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 14 potential issue(s) to consider:
> - [High] Early HID start before driver data is set causes NULL pointer dereference in event handler.
> - [High] Explicitly unregistering and freeing a devm-allocated input device violates input subsystem rules and causes double-frees.
> - [High] LEDs registered with devm tied to `input->dev` lead to Use-After-Free when the parent `ns2` structure is dynamically freed.
> - [High] USB bulk input pipeline permanently stalls if URBs complete before the workqueue processes them.
> - [High] Lockless modification of `urb->state` before unanchoring in USB output error path causes UAF and DMA corruption.
> - [High] Concurrent driver unbind causes a Use-After-Free of `struct switch2_controller`.
> - [High] Teardown ordering in `switch2_usb_disconnect()` frees `bulk_out` URBs before cancelling `message_in_work`.
> - [High] Missing `cancel_work_sync()` in `switch2_usb_probe` error path leads to UAF of `ns2_usb`.
> - [High] Calling `hid_hw_close(hdev)` after `hid_hw_open(hdev)` fails corrupts `ll_open_count`.
> - [High] `usb_free_coherent()` is called while holding a spinlock with IRQs disabled.
> - [Medium] Calling `ida_free()` with an unallocated player ID (`U32_MAX`) triggers a kernel warning.
> - [Medium] Leaking OUT URBs on memory allocation failure in `switch2_usb_probe`.
> - [Medium] Mixing `goto` error handling with `guard()` cleanup violates subsystem anti-pattern rules.
> - [Low] Calling `usb_make_path()` introduces an undeclared hard dependency on the USB core.
> --
>
> commit 322f53e1d34ab62127ead48da7aeff5c1763e2a2
> Author: Vicki Pfau <vi@endrift.com>
>
> HID: nintendo: Add preliminary Switch 2 controller driver
>
> This commit introduces support for the Nintendo Switch 2 Joy-Cons,
> Pro Controller, and GameCube controllers. It implements a split-interface
> design using a main HID driver and a secondary configuration interface.
> Initial support handles general input and USB communications.
>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -843,10 +843,13 @@ config HID_NINTENDO
>> depends on LEDS_CLASS
>> select POWER_SUPPLY
>
> [Severity: Low]
> Does this new configuration introduce an undeclared dependency?
>
> The newly added switch2_probe() function in hid-nintendo.c calls
> usb_make_path(), which is an exported symbol from the USB core. However,
> CONFIG_HID_NINTENDO does not specify depends on USB.
No it's not. It's a static inline function in a header and has been since 2.4.20 over 20 years ago.
>
> If the kernel is built with CONFIG_HID_NINTENDO=y and CONFIG_USB=m, will
> the build fail during linking due to an unresolved reference to
> usb_make_path?>
>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>> --- a/drivers/hid/hid-nintendo.c
>> +++ b/drivers/hid/hid-nintendo.c
> [ ... ]
>> +static int switch2_init_input(struct switch2_controller *ns2)
>> +{
>> + struct input_dev *input;
> [ ... ]
>> + input = devm_input_allocate_device(&hdev->dev);
>> + if (!input)
>> + return -ENOMEM;
>> +
>> + input_set_drvdata(input, ns2);
>> + input->dev.parent = &hdev->dev;
>
> [Severity: High]
> Does explicitly unregistering and freeing a devm-allocated input device
> violate input subsystem rules and cause double-frees?
>
> The driver allocates the input device using devm_input_allocate_device().
> The input subsystem automatically handles cleanup when the provider device is
> unbound. However, the driver explicitly calls input_free_device() on failure
> paths in switch2_init_input() and input_unregister_device() in its teardown
> path switch2_controller_put().
>
> Can this cause a double-free or double-unregister when the devres callbacks
> later attempt to clean up the same device? Additionally, is manually setting
> input->dev.parent redundant and prohibited for devm-allocated input devices?
I've already explained why this is wrong.
>
> [ ... ]
>> + for (i = 0; i < JC_NUM_LEDS; i++) {
>> + struct led_classdev *led = &ns2->leds[i];
> [ ... ]
>> + ret = devm_led_classdev_register(&input->dev, led);
>> + if (ret < 0) {
>> + dev_err(&input->dev, "Failed to register player %d LED; ret=%d\n",
>> + i + 1, ret);
>> + input_unregister_device(input);
>> + return ret;
>> + }
>> + }
>
> [Severity: High]
> Can tying the devm-managed LEDs to input->dev lead to a Use-After-Free
> when the parent ns2 structure is dynamically freed?
>
> The LEDs are registered using devm_led_classdev_register() with input->dev.
> The ns2 structure containing the LEDs is manually freed via kfree(ns2) in
> switch2_controller_put().
>
> The devres callbacks for input->dev execute when its refcount drops to zero.
> If userspace holds an open file descriptor to the input device, this
> cleanup can be arbitrarily delayed.
>
> Will the driver free ns2 immediately while the LEDs remain registered and
> pointing to freed memory until the fd is closed, resulting in a
> Use-After-Free if sysfs LED attributes are accessed?
While there is an exceedingly narrow window for this I think it is theoretically possible. I'll just treat issues creating the LEDs as non-fatal and continue onwards. They can get torn down at the end like everything else.
>
> [ ... ]
>> +static void switch2_controller_put(struct switch2_controller *ns2)
>> +{
>> + struct input_dev *input;
>> + bool do_free;
>> +
>> + guard(mutex)(&switch2_controllers_lock);
>> + mutex_lock(&ns2->lock);
>> +
>> + rcu_read_lock();
>> + input = rcu_dereference(ns2->input);
>> + rcu_read_unlock();
>> +
>> + rcu_assign_pointer(ns2->input, NULL);
>> + synchronize_rcu();
>> +
>> + ns2->init_step = 0;
>> + do_free = !ns2->hdev && !ns2->cfg;
>> + mutex_unlock(&ns2->lock);
>> +
>> + if (input)
>> + input_unregister_device(input);
>> +
>> + if (do_free) {
>> + list_del_init(&ns2->entry);
>> + mutex_destroy(&ns2->lock);
>> + kfree(ns2);
>> + }
>> +}
>
> [Severity: High]
> Could a concurrent driver unbind cause a Use-After-Free of the ns2 structure?
>
> The guard(mutex)(&switch2_controllers_lock) holds the global lock for the
> entire function scope. If HID and USB disconnect concurrently, both clear
> their pointers and call switch2_controller_put().
>
> If the first thread evaluates do_free as true, destroys the mutex, frees
> ns2, and returns, it releases the global lock.
>
> Will the second thread, which was blocked waiting on the global lock,
> wake up and immediately call mutex_lock(&ns2->lock) using the pointer
> that was just freed?
I'm unsure if that re-entry is possible from an external subsystem perspective, but if it is then this narrow hole also exists. I'll convert this to just a basic ref count.
>
> [ ... ]
>> +static int switch2_event(struct hid_device *hdev, struct hid_report *report, uint8_t *raw_data,
>> + int size)
>> +{
>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>> + struct input_dev *input;
>> +
>> + if (report->type != HID_INPUT_REPORT)
>> + return 0;
>> +
>> + if (size < 15)
>> + return -EINVAL;
>> +
>> + guard(rcu)();
>> + input = rcu_dereference(ns2->input);
>
> [Severity: High]
> Could this event handler dereference uninitialized driver data during early
> hardware I/O startup?
>
> The switch2_probe() function calls hid_hw_start(hdev, HID_CONNECT_HIDRAW)
> before assigning its private data with hid_set_drvdata(). This connects the
> device to hidraw, allowing userspace to immediately open it and trigger input
> events, or the underlying transport can send events.
>
> If an event arrives during this race window, will switch2_event() call
> hid_get_drvdata(hdev) and receive a NULL pointer, leading to a kernel crash
> when calling rcu_dereference(ns2->input)?
Seems I've misunderstood the difference between hid_hw_start and hid_hw_open. Fixing in v7.
>
> [ ... ]
>> +int switch2_receive_command(struct switch2_controller *ns2,
>> + const uint8_t *message, size_t length)
>> +{
>> + const struct switch2_cmd_header *header;
>> + int ret = 0;
>> +
>> + if (length < 8)
>> + return -EINVAL;
>> +
>> + print_hex_dump_debug("got cmd: ", DUMP_PREFIX_OFFSET, 16, 1, message, length, false);
>> +
>> + guard(mutex)(&ns2->lock);
>> +
>> + header = (const struct switch2_cmd_header *)message;
>> + if (!(header->flags & NS2_FLAG_OK)) {
>> + dev_warn(ns2->cfg->dev, "Packet error %02x replying to command %x:%x",
>> + header->flags, header->command, header->subcommand);
>> + ret = -EIO;
>> + goto exit;
>> + }
> [ ... ]
>> +exit:
>> + if (ns2->init_step < NS2_INIT_DONE)
>> + switch2_init_controller(ns2);
>> +
>> + return ret;
>> +}
>
> [Severity: Medium]
> Does mixing goto error handling with guard() cleanup violate subsystem rules?
>
> This function utilizes guard(mutex)(&ns2->lock) to acquire the lock using
> scope-based cleanup, but later uses goto exit to jump to a cleanup label
> on error paths.
>
> The core kernel cleanup guidelines explicitly forbid mixing goto-based
> cleanup labels and guard() declarations in the same routine, as it
> creates confusing ownership semantics.
Ah, I guess I misunderstood the how this was forbidden. Easy enough to clean up now that I have a single exit.>
> [ ... ]
>> +static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> +{
> [ ... ]
>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>> + if (ret) {
>> + hid_err(hdev, "hw_start failed %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ns2 = switch2_get_controller(phys);
> [ ... ]
>> + ns2->player_id = U32_MAX;
>> + ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
>> + if (ret < 0)
>> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
>> + else
>> + ns2->player_id = ret;
> [ ... ]
>> + hid_set_drvdata(hdev, ns2);
>> +
>> + ret = hid_hw_open(hdev);
>> + if (ret) {
>> + hid_err(hdev, "hw_open failed %d\n", ret);
>> + goto err_cleanup;
>> + }
>> +
>> + ret = 0;
>> + if (ns2->cfg)
>> + ret = switch2_init_controller(ns2);
>> +
>> + if (!ret) {
>> + mutex_unlock(&ns2->lock);
>> + return 0;
>> + }
>> +
>> +err_cleanup:
>> + hid_hw_close(hdev);
>> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
>> + ns2->hdev = NULL;
>> + mutex_unlock(&ns2->lock);
>
> [Severity: High]
> If hid_hw_open() fails, does the unconditional call to hid_hw_close(hdev)
> corrupt the ll_open_count tracking?
>
> If hid_hw_open(hdev) fails, execution jumps to the err_cleanup label, which
> unconditionally calls hid_hw_close(hdev).
>
> Since hid_hw_open() does not increment ll_open_count on failure, will
> hid_hw_close() decrement the count from 0, wrapping it around to UINT_MAX
> and permanently silently preventing any future opens of the device?
Fixing as part of other cleanup.
>
> [Severity: Medium]
> Does this error path trigger a WARN_ON when trying to free an unallocated
> player ID?
>
> The code initializes ns2->player_id to U32_MAX. If ida_alloc() fails,
> it skips allocation but retains the U32_MAX value.
>
> In the err_cleanup path, or during switch2_remove(), the driver
> unconditionally calls ida_free(&nintendo_player_id_allocator, ns2->player_id).
>
> Will passing -1 (U32_MAX) to ida_free() trigger a WARN_ON within the IDA
> framework?
Already shown to be a hallucination in the last reply.
>
>> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
>> --- /dev/null
>> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
> [ ... ]
>> +static void switch2_bulk_in(struct urb *urb)
>> +{
> [ ... ]
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + for (i = 0; i < NS2_IN_URBS; i++) {
>> + int err;
>> + struct switch2_urb *ns2_urb;
>> +
>> + if (ns2_usb->bulk_in[i].urb == urb) {
>> + ns2_usb->bulk_in[i].state = NS2_URB_IN;
>> + continue;
>> + }
>> +
>> + if (ns2_usb->bulk_in[i].state != NS2_URB_FREE)
>> + continue;
>> +
>> + ns2_urb = &ns2_usb->bulk_in[i];
>> + usb_anchor_urb(ns2_urb->urb, &ns2_usb->bulk_in_anchor);
>> + err = usb_submit_urb(ns2_urb->urb, GFP_ATOMIC);
>> + if (err) {
>> + usb_unanchor_urb(ns2_urb->urb);
>> + dev_dbg(&ns2_usb->udev->dev, "failed to queue input urb: %d\n", err);
>> + } else {
>> + ns2_urb->state = NS2_URB_OUT;
>> + }
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + if (schedule)
>> + schedule_work(&ns2_usb->message_in_work);
>> +}
> [ ... ]
>> +static void switch2_usb_message_in_work(struct work_struct *work)
>> +{
> [ ... ]
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + urb->state = NS2_URB_FREE;
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +}
>
> [Severity: High]
> Does this input URB handling stall the USB bulk input pipeline permanently
> if the URBs complete too quickly?
>
> The driver manages IN URBs via a ping-pong mechanism. switch2_bulk_in()
> submits URBs in the NS2_URB_FREE state. The worker
> switch2_usb_message_in_work() clears processed URBs to NS2_URB_FREE but
> does not re-submit them.
>
> If all active URBs complete in quick succession before the workqueue
> thread runs, they will all be marked NS2_URB_IN. The worker will process
> them and mark them FREE, but since no URBs are left active in the USB core
> to trigger the completion handler, will switch2_bulk_in() never be called
> again, permanently halting input?
In theory, the Switch 2 controller should only send a bulk IN in reply to a bulk OUT, and we don't tend to schedule more than one of those at a time, but it'd be good to fix this anyway.>
> [ ... ]
>> +static int switch2_usb_send_cmd(enum switch2_cmd command, uint8_t subcommand,
>> + const void *message, size_t size, struct switch2_cfg_intf *cfg)
>> +{
> [ ... ]
>> + usb_anchor_urb(urb->urb, &ns2_usb->bulk_out_anchor);
>> + ret = usb_submit_urb(urb->urb, GFP_ATOMIC);
>> + if (ret) {
>> + if (ret != -ENODEV)
>> + dev_warn(&ns2_usb->udev->dev, "failed to submit output urb: %i", ret);
>> + urb->state = NS2_URB_FREE;
>> + usb_unanchor_urb(urb->urb);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>
> [Severity: High]
> Can this lockless modification of urb->state lead to a data race and
> potential DMA corruption?
>
> If usb_submit_urb() fails, urb->state = NS2_URB_FREE is set without
> re-acquiring ns2_usb->bulk_out_lock. Additionally, it is marked FREE
> before calling usb_unanchor_urb().
>
> Could a concurrent thread acquire the lock, claim this supposedly free
> URB, write to its DMA buffer, and submit it, right as the original
> thread unanchors it? During teardown, usb_kill_anchored_urbs() would not
> wait for this unanchored URB, potentially leading to its DMA buffer being
> freed while the hardware is still accessing it.
Missed this. Fixing in v7. Wish the stochastic parrot had found this on v5.
>
> [ ... ]
>> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
>> +{
> [ ... ]
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + struct urb *urb;
>> + uint8_t *data;
>> +
>> + urb = usb_alloc_urb(0, GFP_KERNEL);
>> + if (!urb) {
>> + ret = -ENOMEM;
>> + goto err_free_in;
>> + }
> [ ... ]
>> + ret = switch2_controller_attach_cfg(phys, &ns2_usb->cfg);
>> + if (ret < 0)
>> + goto err_free_out;
>> +
>> + return 0;
>> +
>> +err_free_out:
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + if (!ns2_usb->bulk_out[i].urb)
>> + continue;
>> +
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
>> + ns2_usb->bulk_out[i].urb->transfer_dma);
>> + usb_free_urb(ns2_usb->bulk_out[i].urb);
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> +err_free_in:
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
> [ ... ]
>> + devm_kfree(&intf->dev, ns2_usb);
>> +
>> + return ret;
>> +}
>
> [Severity: Medium]
> Does this error handling skip cleaning up successfully allocated OUT URBs?
>
> During the loop allocating OUT URBs, if usb_alloc_urb() fails, the code
> branches to goto err_free_in. This skips the err_free_out block entirely.
>
> Will this permanently leak the OUT URBs and their DMA buffers from
> any preceding successful loop iterations?
Fixed.
>
> [Severity: High]
> Does calling usb_free_coherent() inside the atomic critical section violate
> DMA API rules?
>
> In the err_free_out block, usb_free_coherent() is called while the
> bulk_out_lock spinlock is held with IRQs disabled (using spin_lock_irqsave).
>
> Since usb_free_coherent() is a wrapper for dma_free_coherent() and freeing
> memory allocated with GFP_KERNEL may sleep, will this cause deadlocks or
> kernel panics under CONFIG_DEBUG_ATOMIC_SLEEP?
Fixed.
>
> [Severity: High]
> Is a cancel_work_sync() missing from this error path, potentially causing
> a Use-After-Free?
>
> An IN URB is submitted before calling switch2_controller_attach_cfg().
> If the device responds immediately, switch2_bulk_in() will schedule
> ns2_usb->message_in_work.
>
> If attach_cfg fails, the error path frees ns2_usb via devm_kfree() but
> never calls cancel_work_sync(&ns2_usb->message_in_work).
>
> Could the scheduled work execute later on the freed memory footprint of
> ns2_usb?
Fixed.
>
> [ ... ]
>> +static void switch2_usb_disconnect(struct usb_interface *intf)
>> +{
>> + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
>> + unsigned long flags;
>> + int i;
>> +
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
>> + ns2_usb->bulk_out[i].urb->transfer_dma);
>> + usb_free_urb(ns2_usb->bulk_out[i].urb);
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> +
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
>> + cancel_work_sync(&ns2_usb->message_in_work);
>
> [Severity: High]
> Does freeing the bulk_out URBs before cancelling the worker lead to
> a Use-After-Free and DMA corruption?
>
> All bulk_out URBs and their DMA buffers are freed before the driver
> invokes cancel_work_sync(&ns2_usb->message_in_work).
>
> Can the globally scheduled workqueue run concurrently during this gap,
> process incoming messages, and trigger a reply via switch2_usb_send_cmd()?
>
> If so, will the send function claim a freed URB and write to the freed DMA
> buffer, leading to a Use-After-Free and submitting corrupted DMA to the
> USB controller?
>
> [Severity: High]
> Does calling usb_free_coherent() inside the atomic critical section violate
> DMA API rules?
>
> Similar to the probe error path, usb_free_coherent() is called while the
> bulk_out_lock spinlock is held with IRQs disabled (using spin_lock_irqsave).
>
> Since freeing memory allocated with GFP_KERNEL may sleep, will this cause
> deadlocks or kernel panics under CONFIG_DEBUG_ATOMIC_SLEEP?
>
Fixed a bunch of things in this too.
I'm noticing a bunch of ordering stuff in this file that's rough to get right. Hopefully I've fixed it now and will be submitting v7 on Monday probably. Still need to review patch 2 and retest everything.
^ permalink raw reply
* Re: [PATCH v6 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: Vicki Pfau @ 2026-06-27 2:48 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260625051833.C1A131F000E9@smtp.kernel.org>
On 6/24/26 10:18 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Unsynchronized accesses to ns2->hdev allow rumble work to read a closing device pointer, leading to a TOCTOU NULL pointer dereference.
> - [High] cancel_delayed_work_sync() is called on an uninitialized work structure if the configuration interface detaches without the HID interface probing.
> - [High] Use-after-free of ns2 due to the input core's memless force feedback timer outliving the driver's private data.
> - [Medium] Arithmetic underflow in schedule_delayed_work() delay calculation causes the rumble worker to permanently hang if its execution is delayed.
> - [Low] Unused static constant array switch2_zero_data leads to dead code.
> --
>
> commit 93841885eb908e1356dc311955f33fee3d1428c6
> Author: Vicki Pfau <vi@endrift.com>
>
> HID: nintendo: Add rumble support for Switch 2 controllers
>
> This commit adds basic rumble support for the Switch 2 controllers. It
> introduces delayed work and force feedback support.
>
>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>> index 6573fd31a232a..ae360f4d15d1a 100644
>> --- a/drivers/hid/hid-nintendo.c
>> +++ b/drivers/hid/hid-nintendo.c
>
> [ ... ]
>
>> @@ -3133,9 +3183,12 @@ static const uint8_t switch2_init_cmd_data[] = {
>> };
>>
>> static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
>> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
>> +static const uint8_t switch2_zero_data[] = { 0x00, 0x00, 0x00, 0x00 };
>> +#endif
>
> [Severity: Low]
> Is this unused variable intentional? It appears switch2_zero_data is never
> utilized anywhere in the codebase.
Looks like leftover from a previous iteration. Removing.
>
> [ ... ]
>
>> +static int switch2_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
>> +{
>> + struct switch2_controller *ns2 = input_get_drvdata(dev);
>> +
>> + if (effect->type != FF_RUMBLE)
>> + return 0;
>> +
>> + guard(spinlock_irqsave)(&ns2->rumble_lock);
>
> [Severity: High]
> Can this result in a use-after-free when the input device outlives the driver
> data?
>
> During device removal, switch2_controller_put() explicitly calls kfree(ns2).
> However, the input device is devres-managed and its lifetime is tied to
> userspace holding its file descriptor open. Since the input core's memless
> force feedback timer is only destroyed when the input device is released,
> the timer callback switch2_play_effect() could potentially fire after ns2
> has been freed.
I will see about tying this into the ref system I added for patch 1 to delay the free.>
> [ ... ]
>
>> +static void switch2_rumble_work(struct work_struct *work)
>> +{
>
> [ ... ]
>
>> + if (active) {
>> + unsigned long interval = msecs_to_jiffies(2);
>> + uint64_t current_jiffies = get_jiffies_64();
>> +
>> + if (!ns2->last_rumble_work)
>> + ns2->last_rumble_work = current_jiffies;
>> + else
>> + ns2->last_rumble_work += interval;
>> + schedule_delayed_work(&ns2->rumble_work,
>> + ns2->last_rumble_work + interval - current_jiffies);
>
> [Severity: Medium]
> Could this delay calculation underflow?
>
> If the worker thread is delayed by the scheduler such that current_jiffies
> exceeds the expected execution time (ns2->last_rumble_work + interval), the
> unsigned arithmetic could underflow to a value near ULONG_MAX. This would
> schedule the next rumble update weeks in the future, permanently hanging the
> force feedback mechanism.
I think I've seen this. I have trouble reproducing it but I'll fix this and hopefully it'll stop reproducing.
>
>> + } else {
>> + ns2->last_rumble_work = 0;
>> + }
>> + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
>> +
>> + if (!ns2->hdev)
>> + cancel_delayed_work(&ns2->rumble_work);
>> + else
>> + ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
>
> [Severity: High]
> Is it possible for a concurrent removal to cause a NULL pointer dereference
> here?
>
> Because ns2->hdev is read outside of any lock, there may be a window where it
> is verified as non-NULL, but then set to NULL by a concurrent
> switch2_remove() before being passed to hid_hw_output_report(). This appears
> structurally possible if userspace invokes switch2_play_effect() and
> reschedules the worker right after the initial cancellation in
> switch2_remove().
>
> [ ... ]
Fixing.
>
>> @@ -3358,6 +3521,10 @@ static void switch2_controller_put(struct switch2_controller *ns2)
>> if (input)
>> input_unregister_device(input);
>>
>> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
>> + cancel_delayed_work_sync(&ns2->rumble_work);
>> +#endif
>
> [Severity: High]
> Could this cancel operation act on an uninitialized work structure?
>
> The switch2_probe() function initializes the work structure:
>
> INIT_DELAYED_WORK(&ns2->rumble_work, switch2_rumble_work);
>
> If the configuration interface detaches before the HID interface probes,
> ns2->rumble_work might remain zero-initialized. When switch2_controller_put()
> is called, unconditionally invoking cancel_delayed_work_sync() on this
> uninitialized work structure could trigger lockdep warnings and kernel panics.
>
This will be fixed by my probe refactor already.
^ permalink raw reply
* Re: [PATCH] Input: sur40: fix MAX_CONTACTS value based on PixelSense specification
From: Dmitry Torokhov @ 2026-06-27 5:25 UTC (permalink / raw)
To: Oliver; +Cc: kees, linux-input, linux-kernel
In-Reply-To: <20260614230847.4938-1-oliverburns.kernel@gmail.com>
On Mon, Jun 15, 2026 at 12:08:47AM +0100, Oliver wrote:
> The Samsung SUR40 with Microsoft PixelSense is offically specified to
> support 52 simultaneuous touch contacts, not 64. The value of 64 was an
> unverified guess as noted by the FIXME comment. Update MAX_CONTACTS to
> match the documented hardware specification and remove the FIXME.
>
> Signed-off-by: Oliver <oliverburns.kernel@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: gscps2 - advance receive buffer write index
From: Dmitry Torokhov @ 2026-06-27 5:43 UTC (permalink / raw)
To: raoxu
Cc: James.Bottomley, deller, linux-parisc, linux-input, linux-kernel,
stable
In-Reply-To: <460B5655BA580C60+20260624094739.850306-1-raoxu@uniontech.com>
On Wed, Jun 24, 2026 at 05:47:39PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> Commit 44f920069911 ("Input: gscps2 - use guard notation when
> acquiring spinlock") moved the receive loop into gscps2_read_data()
> and gscps2_report_data().
>
> While moving the code, it preserved the writes to
> buffer[ps2port->append], but omitted the following producer index
> update from the original loop:
>
> ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
>
> As a result, append never advances. Since gscps2_report_data() only
> reports bytes while act != append, the receive buffer always appears
> empty and no keyboard or mouse data reaches the serio core.
>
> Restore the omitted index update.
>
> Fixes: 44f920069911 ("Input: gscps2 - use guard notation when acquiring spinlock")
> Cc: stable@vger.kernel.org # 6.13+
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [regression] synaptics-rmi4: Failed to read the Control Register Descriptor: EIO
From: David Heidelberg @ 2026-06-27 10:06 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Linux Next Mailing List
In-Reply-To: <aj7saJXgFX--gP_8@google.com>
On 26/06/2026 23:18, Dmitry Torokhov wrote:
> Hi David,
>
> On Fri, Jun 26, 2026 at 11:09:54PM +0200, David Heidelberg wrote:
>> With next-20260626 without any additional patches, the synaptics fails on
>> OnePlus 6T with:
>>
>> ```
>> [ 16.620292] rmi4_i2c 12-0020: registering I2C-connected sensor
>> [ 16.857617] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer:
>> Synaptics, product: S3706B, fw id: 2852315
>> [ 16.867324] rmi4_f12 rmi4-00.fn12: Failed to read the Control Register
>> Descriptor: -5
>> [ 16.867338] rmi4_f12 rmi4-00.fn12: probe with driver rmi4_f12 failed with
>> error -5
>> ```
>
> Yes, at least a98518e72439 "Input: rmi4 - fix register descriptor address
> calculation" is wrong and needs to be reverted.
That did it for me, now the TS works again.
Thank you
David
^ permalink raw reply
* [PATCH] Input: rmi4 - Move sensor initialization into own function for F12
From: David Heidelberg via B4 Relay @ 2026-06-27 14:56 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, David Heidelberg
From: David Heidelberg <david@ixit.cz>
Makes rmi_f12_probe function a bit cleaner and more structured.
Signed-off-by: David Heidelberg <david@ixit.cz>
---
drivers/input/rmi4/rmi_f12.c | 58 ++++++++++++++++++++++++++------------------
1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 88c28089de993..333477bf0663e 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -364,29 +364,60 @@ static int rmi_f12_config(struct rmi_function *fn)
ret = rmi_f12_write_control_regs(fn);
if (ret)
dev_warn(&fn->dev,
"Failed to write F12 control registers: %d\n", ret);
return 0;
}
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+ size_t pkt_size;
+
+ sensor->fn = fn;
+ f12->data_addr = fn->fd.data_base_addr;
+ pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+ if (pkt_size > SZ_1M) {
+ dev_err(&fn->dev, "Invalid data packet size: %zu\n", pkt_size);
+ return -EINVAL;
+ }
+ sensor->pkt_size = pkt_size;
+
+ sensor->axis_align = f12->sensor_pdata.axis_align;
+
+ sensor->x_mm = f12->sensor_pdata.x_mm;
+ sensor->y_mm = f12->sensor_pdata.y_mm;
+ sensor->dribble = f12->sensor_pdata.dribble;
+
+ if (sensor->sensor_type == rmi_sensor_default)
+ sensor->sensor_type = f12->sensor_pdata.sensor_type;
+
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %u\n", __func__,
+ sensor->pkt_size);
+ sensor->data_pkt = devm_kmalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+ if (!sensor->data_pkt)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int rmi_f12_probe(struct rmi_function *fn)
{
struct f12_data *f12;
int ret;
struct rmi_device *rmi_dev = fn->rmi_dev;
char buf;
u16 query_addr = fn->fd.query_base_addr;
const struct rmi_register_desc_item *item;
struct rmi_2d_sensor *sensor;
struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
size_t data_offset = 0;
- size_t pkt_size;
int irq_mask_size;
int i;
rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
irq_mask_size = BITS_TO_LONGS(drvdata->irq_count);
ret = rmi_read(fn->rmi_dev, query_addr, &buf);
@@ -421,39 +452,20 @@ static int rmi_f12_probe(struct rmi_function *fn)
f12->sensor_pdata = pdata->sensor_pdata;
}
ret = rmi_f12_read_register_descs(fn, f12, query_addr);
if (ret)
return ret;
sensor = &f12->sensor;
- sensor->fn = fn;
- f12->data_addr = fn->fd.data_base_addr;
- pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
- if (pkt_size > SZ_1M) {
- dev_err(&fn->dev, "Invalid data packet size: %zu\n", pkt_size);
- return -EINVAL;
- }
- sensor->pkt_size = pkt_size;
-
- sensor->axis_align = f12->sensor_pdata.axis_align;
- sensor->x_mm = f12->sensor_pdata.x_mm;
- sensor->y_mm = f12->sensor_pdata.y_mm;
- sensor->dribble = f12->sensor_pdata.dribble;
-
- if (sensor->sensor_type == rmi_sensor_default)
- sensor->sensor_type = f12->sensor_pdata.sensor_type;
-
- rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %u\n", __func__,
- sensor->pkt_size);
- sensor->data_pkt = devm_kmalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
- if (!sensor->data_pkt)
- return -ENOMEM;
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
dev_set_drvdata(&fn->dev, f12);
ret = rmi_f12_read_sensor_tuning(f12);
if (ret)
return ret;
/*
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260627-synaptics-rmi4-fnsplit-3698a620b73c
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply related
* [dtor-input:next] BUILD SUCCESS c8f174900926d3b58cd048ac33b4cbb3de419bfe
From: kernel test robot @ 2026-06-27 18:22 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: c8f174900926d3b58cd048ac33b4cbb3de419bfe Input: sur40 - fix MAX_CONTACTS value based on PixelSense specification
elapsed time: 730m
configs tested: 213
configs skipped: 11
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
alpha defconfig gcc-16.1.0
arc allmodconfig gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig gcc-16.1.0
arc defconfig gcc-16.1.0
arc randconfig-001-20260627 gcc-8.5.0
arc randconfig-002-20260627 gcc-8.5.0
arm allnoconfig clang-23
arm allyesconfig gcc-16.1.0
arm defconfig clang-23
arm pxa910_defconfig gcc-16.1.0
arm randconfig-001-20260627 gcc-16.1.0
arm randconfig-002-20260627 clang-23
arm randconfig-003-20260627 gcc-16.1.0
arm randconfig-004-20260627 clang-19
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
arm64 defconfig gcc-16.1.0
arm64 randconfig-001 gcc-14.3.0
arm64 randconfig-001-20260627 gcc-10.5.0
arm64 randconfig-002 gcc-8.5.0
arm64 randconfig-002-20260627 clang-23
arm64 randconfig-003 clang-20
arm64 randconfig-003-20260627 gcc-10.5.0
arm64 randconfig-004 clang-23
arm64 randconfig-004-20260627 gcc-16.1.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky defconfig gcc-16.1.0
csky randconfig-001 gcc-13.4.0
csky randconfig-001-20260627 gcc-16.1.0
csky randconfig-002 gcc-14.3.0
csky randconfig-002-20260627 gcc-14.3.0
hexagon allmodconfig clang-23
hexagon allnoconfig clang-23
hexagon defconfig clang-23
hexagon randconfig-001 clang-23
hexagon randconfig-001-20260627 clang-23
hexagon randconfig-002 clang-23
hexagon randconfig-002-20260627 clang-18
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260627 clang-22
i386 buildonly-randconfig-002-20260627 clang-22
i386 buildonly-randconfig-003-20260627 clang-22
i386 buildonly-randconfig-004-20260627 clang-22
i386 buildonly-randconfig-005-20260627 gcc-13
i386 buildonly-randconfig-006-20260627 clang-22
i386 defconfig clang-22
i386 randconfig-001 clang-22
i386 randconfig-001-20260627 clang-22
i386 randconfig-002 gcc-14
i386 randconfig-002-20260627 clang-22
i386 randconfig-003 gcc-14
i386 randconfig-003-20260627 gcc-13
i386 randconfig-004 clang-22
i386 randconfig-004-20260627 clang-22
i386 randconfig-005 gcc-14
i386 randconfig-005-20260627 clang-22
i386 randconfig-006 gcc-14
i386 randconfig-006-20260627 gcc-14
i386 randconfig-007 gcc-14
i386 randconfig-007-20260627 clang-22
i386 randconfig-011-20260627 clang-22
i386 randconfig-012-20260627 clang-22
i386 randconfig-013-20260627 gcc-14
i386 randconfig-014-20260627 clang-22
i386 randconfig-015-20260627 gcc-14
i386 randconfig-016-20260627 clang-22
i386 randconfig-017-20260627 gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-20
loongarch defconfig clang-23
loongarch randconfig-001 clang-23
loongarch randconfig-001-20260627 clang-23
loongarch randconfig-002 gcc-16.1.0
loongarch randconfig-002-20260627 gcc-16.1.0
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig gcc-16.1.0
m68k defconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
microblaze defconfig gcc-16.1.0
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
nios2 defconfig gcc-11.5.0
nios2 randconfig-001 gcc-11.5.0
nios2 randconfig-001-20260627 gcc-11.5.0
nios2 randconfig-002 gcc-8.5.0
nios2 randconfig-002-20260627 gcc-11.5.0
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig gcc-16.1.0
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig gcc-16.1.0
parisc allyesconfig gcc-16.1.0
parisc defconfig gcc-16.1.0
parisc randconfig-001 gcc-8.5.0
parisc randconfig-001-20260627 gcc-8.5.0
parisc randconfig-002 gcc-12.5.0
parisc randconfig-002-20260627 gcc-8.5.0
parisc64 defconfig gcc-16.1.0
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig gcc-16.1.0
powerpc randconfig-001 gcc-10.5.0
powerpc randconfig-001-20260627 clang-23
powerpc randconfig-002 gcc-8.5.0
powerpc randconfig-002-20260627 clang-17
powerpc64 randconfig-001 clang-16
powerpc64 randconfig-001-20260627 clang-20
powerpc64 randconfig-002 clang-23
powerpc64 randconfig-002-20260627 clang-23
riscv allmodconfig clang-23
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-23
riscv defconfig clang-23
riscv randconfig-001-20260627 clang-22
riscv randconfig-002-20260627 clang-23
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
s390 defconfig clang-18
s390 randconfig-001-20260627 clang-23
s390 randconfig-002-20260627 clang-23
sh allmodconfig gcc-16.1.0
sh allnoconfig gcc-16.1.0
sh allyesconfig gcc-16.1.0
sh defconfig gcc-16.1.0
sh randconfig-001-20260627 gcc-16.1.0
sh randconfig-002-20260627 gcc-10.5.0
sh se7724_defconfig gcc-16.1.0
sparc allnoconfig gcc-16.1.0
sparc defconfig gcc-16.1.0
sparc randconfig-001 gcc-16.1.0
sparc randconfig-001-20260627 gcc-13.4.0
sparc randconfig-002 gcc-11.5.0
sparc randconfig-002-20260627 gcc-8.5.0
sparc64 allmodconfig clang-20
sparc64 defconfig clang-23
sparc64 randconfig-001 gcc-12.5.0
sparc64 randconfig-001-20260627 gcc-13.4.0
sparc64 randconfig-002 clang-20
sparc64 randconfig-002-20260627 gcc-15.2.0
um allmodconfig clang-23
um allnoconfig clang-16
um allyesconfig gcc-14
um defconfig clang-23
um i386_defconfig gcc-14
um randconfig-001 clang-23
um randconfig-001-20260627 clang-22
um randconfig-002 clang-23
um randconfig-002-20260627 clang-23
um x86_64_defconfig clang-23
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-22
x86_64 allyesconfig clang-22
x86_64 buildonly-randconfig-001 gcc-12
x86_64 buildonly-randconfig-001-20260627 gcc-13
x86_64 buildonly-randconfig-002 clang-22
x86_64 buildonly-randconfig-002-20260627 clang-22
x86_64 buildonly-randconfig-003 gcc-14
x86_64 buildonly-randconfig-003-20260627 clang-22
x86_64 buildonly-randconfig-004 gcc-14
x86_64 buildonly-randconfig-004-20260627 gcc-14
x86_64 buildonly-randconfig-005 gcc-14
x86_64 buildonly-randconfig-005-20260627 clang-22
x86_64 buildonly-randconfig-006 clang-22
x86_64 buildonly-randconfig-006-20260627 clang-22
x86_64 defconfig gcc-14
x86_64 randconfig-001 gcc-14
x86_64 randconfig-001-20260627 clang-22
x86_64 randconfig-002 gcc-14
x86_64 randconfig-002-20260627 gcc-12
x86_64 randconfig-003 clang-22
x86_64 randconfig-003-20260627 clang-22
x86_64 randconfig-004 clang-22
x86_64 randconfig-004-20260627 gcc-14
x86_64 randconfig-005 gcc-14
x86_64 randconfig-005-20260627 gcc-14
x86_64 randconfig-006 clang-22
x86_64 randconfig-006-20260627 clang-22
x86_64 randconfig-011 gcc-14
x86_64 randconfig-011-20260627 clang-22
x86_64 randconfig-012 gcc-14
x86_64 randconfig-012-20260627 clang-22
x86_64 randconfig-013 clang-22
x86_64 randconfig-013-20260627 gcc-14
x86_64 randconfig-014 gcc-14
x86_64 randconfig-014-20260627 gcc-14
x86_64 randconfig-015 gcc-14
x86_64 randconfig-015-20260627 gcc-14
x86_64 randconfig-016 clang-22
x86_64 randconfig-016-20260627 gcc-14
x86_64 randconfig-071-20260627 clang-22
x86_64 randconfig-072-20260627 clang-22
x86_64 randconfig-073-20260627 clang-22
x86_64 randconfig-074-20260627 gcc-14
x86_64 randconfig-075-20260627 gcc-14
x86_64 randconfig-076-20260627 clang-22
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig gcc-16.1.0
xtensa allyesconfig gcc-16.1.0
xtensa randconfig-001 gcc-8.5.0
xtensa randconfig-001-20260627 gcc-8.5.0
xtensa randconfig-002 gcc-13.4.0
xtensa randconfig-002-20260627 gcc-8.5.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS d86d4f8cbb5a55a3b9b86f7b5ab8c4cdda600a3f
From: kernel test robot @ 2026-06-27 18:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: d86d4f8cbb5a55a3b9b86f7b5ab8c4cdda600a3f Input: gscps2 - advance receive buffer write index
elapsed time: 737m
configs tested: 166
configs skipped: 12
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
alpha defconfig gcc-16.1.0
arc allmodconfig gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig gcc-16.1.0
arc defconfig gcc-16.1.0
arc randconfig-001-20260627 gcc-8.5.0
arc randconfig-002-20260627 gcc-8.5.0
arm allnoconfig clang-23
arm allyesconfig gcc-16.1.0
arm defconfig clang-23
arm randconfig-001-20260627 gcc-16.1.0
arm randconfig-002-20260627 clang-23
arm randconfig-003-20260627 gcc-16.1.0
arm randconfig-004-20260627 clang-19
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
arm64 defconfig gcc-16.1.0
arm64 randconfig-001-20260627 gcc-10.5.0
arm64 randconfig-002-20260627 clang-23
arm64 randconfig-003-20260627 gcc-10.5.0
arm64 randconfig-004-20260627 gcc-16.1.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky defconfig gcc-16.1.0
csky randconfig-001-20260627 gcc-16.1.0
csky randconfig-002-20260627 gcc-14.3.0
hexagon allmodconfig clang-23
hexagon allnoconfig clang-23
hexagon defconfig clang-23
hexagon randconfig-001-20260627 clang-23
hexagon randconfig-002-20260627 clang-18
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260627 clang-22
i386 buildonly-randconfig-002-20260627 clang-22
i386 buildonly-randconfig-003-20260627 clang-22
i386 buildonly-randconfig-004-20260627 clang-22
i386 buildonly-randconfig-005-20260627 gcc-13
i386 buildonly-randconfig-006-20260627 clang-22
i386 defconfig clang-22
i386 randconfig-001-20260627 clang-22
i386 randconfig-002-20260627 clang-22
i386 randconfig-003-20260627 gcc-13
i386 randconfig-004-20260627 clang-22
i386 randconfig-005-20260627 clang-22
i386 randconfig-006-20260627 gcc-14
i386 randconfig-007-20260627 clang-22
i386 randconfig-011-20260627 clang-22
i386 randconfig-012-20260627 clang-22
i386 randconfig-013-20260627 gcc-14
i386 randconfig-014-20260627 clang-22
i386 randconfig-015-20260627 gcc-14
i386 randconfig-016-20260627 clang-22
i386 randconfig-017-20260627 gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-20
loongarch defconfig clang-23
loongarch randconfig-001-20260627 clang-23
loongarch randconfig-002-20260627 gcc-16.1.0
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig gcc-16.1.0
m68k defconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
microblaze defconfig gcc-16.1.0
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
nios2 defconfig gcc-11.5.0
nios2 randconfig-001-20260627 gcc-11.5.0
nios2 randconfig-002-20260627 gcc-11.5.0
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig gcc-16.1.0
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig gcc-16.1.0
parisc allyesconfig gcc-16.1.0
parisc defconfig gcc-16.1.0
parisc randconfig-001-20260627 gcc-8.5.0
parisc randconfig-002-20260627 gcc-8.5.0
parisc64 defconfig gcc-16.1.0
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig gcc-16.1.0
powerpc randconfig-001-20260627 clang-23
powerpc randconfig-002-20260627 clang-17
powerpc stx_gp3_defconfig gcc-16.1.0
powerpc64 randconfig-001-20260627 clang-20
powerpc64 randconfig-002-20260627 clang-23
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-23
riscv defconfig clang-23
riscv randconfig-001 gcc-13.4.0
riscv randconfig-001-20260627 clang-22
riscv randconfig-002 clang-23
riscv randconfig-002-20260627 clang-23
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
s390 defconfig clang-18
s390 randconfig-001 gcc-15.2.0
s390 randconfig-001-20260627 clang-23
s390 randconfig-002 clang-18
s390 randconfig-002-20260627 clang-23
sh allmodconfig gcc-16.1.0
sh allnoconfig gcc-16.1.0
sh allyesconfig gcc-16.1.0
sh defconfig gcc-16.1.0
sh randconfig-001 gcc-14.3.0
sh randconfig-001-20260627 gcc-16.1.0
sh randconfig-002 gcc-9.5.0
sh randconfig-002-20260627 gcc-10.5.0
sparc allnoconfig gcc-16.1.0
sparc defconfig gcc-16.1.0
sparc randconfig-001 gcc-16.1.0
sparc randconfig-001-20260627 gcc-13.4.0
sparc randconfig-002 gcc-11.5.0
sparc randconfig-002-20260627 gcc-8.5.0
sparc64 allmodconfig clang-20
sparc64 defconfig clang-23
sparc64 randconfig-001 gcc-12.5.0
sparc64 randconfig-002 clang-20
sparc64 randconfig-002-20260627 gcc-15.2.0
um allmodconfig clang-23
um allnoconfig clang-16
um allyesconfig gcc-14
um defconfig clang-23
um i386_defconfig gcc-14
um randconfig-002 clang-23
um x86_64_defconfig clang-23
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-22
x86_64 allyesconfig clang-22
x86_64 buildonly-randconfig-001-20260627 gcc-13
x86_64 buildonly-randconfig-002-20260627 clang-22
x86_64 buildonly-randconfig-003-20260627 clang-22
x86_64 buildonly-randconfig-004-20260627 gcc-14
x86_64 buildonly-randconfig-005-20260627 clang-22
x86_64 buildonly-randconfig-006-20260627 clang-22
x86_64 defconfig gcc-14
x86_64 randconfig-001-20260627 clang-22
x86_64 randconfig-002-20260627 gcc-12
x86_64 randconfig-003-20260627 clang-22
x86_64 randconfig-004-20260627 gcc-14
x86_64 randconfig-005-20260627 gcc-14
x86_64 randconfig-006-20260627 clang-22
x86_64 randconfig-011-20260627 clang-22
x86_64 randconfig-012-20260627 clang-22
x86_64 randconfig-013-20260627 gcc-14
x86_64 randconfig-014-20260627 gcc-14
x86_64 randconfig-015-20260627 gcc-14
x86_64 randconfig-016-20260627 gcc-14
x86_64 randconfig-071-20260627 clang-22
x86_64 randconfig-072-20260627 clang-22
x86_64 randconfig-073-20260627 clang-22
x86_64 randconfig-074-20260627 gcc-14
x86_64 randconfig-075-20260627 gcc-14
x86_64 randconfig-076-20260627 clang-22
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig gcc-16.1.0
xtensa randconfig-001 gcc-8.5.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2] HID: hyperx-headset: Add support for HyperX headset devices
From: Sofia Schneider @ 2026-06-27 20:39 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, Sofia Schneider
Introduce a HID driver for HyperX Cloud III Wireless headsets,
supporting battery reporting and connection status.
Tested with a HyperX Cloud III Wireless only, for lack of
other testable devices.
Signed-off-by: Sofia Schneider <sofia@schn.dev>
---
v2:
- Call hid_hw_stop() before cancelling work queues in remove() to prevent UAF.
- Safe unregistration of power supply using the spinlock in remove() to prevent race conditions.
- Cancel battery_work in the hid_hw_start() error path in probe() to prevent UAF.
- Dynamically allocate power supply names to avoid sysfs naming collisions.
v1: https://lore.kernel.org/linux-input/20260622022800.D01D51F000E9@smtp.kernel.org/T/#t
MAINTAINERS | 6 +
drivers/hid/Kconfig | 11 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-hyperx-headset.c | 387 +++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
5 files changed, 406 insertions(+)
create mode 100644 drivers/hid/hid-hyperx-headset.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d8252026bbd4..fa49655255f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11419,6 +11419,12 @@ F: include/uapi/linux/hid*
F: samples/hid/
F: tools/testing/selftests/hid/
+HID HYPERX HEADSET DRIVER
+M: Sofia Schneider <sofia@schn.dev>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-hyperx-headset.c
+
HID LOGITECH DRIVERS
R: Filipe Laíns <lains@riseup.net>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index f9bcaeb66385..e9f5f1f982c9 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1215,6 +1215,17 @@ config HID_HYPERV_MOUSE
help
Select this option to enable the Hyper-V mouse driver.
+config HID_HYPERX_HEADSET
+ tristate "HyperX headset devices"
+ depends on USB_HID
+ select POWER_SUPPLY
+ help
+ Support for HyperX headset devices.
+
+ Say Y here if you would like to enable support for HyperX headset devices.
+ To compile this driver as a module, choose M here: the module will be called
+ hid-hyperx-headset.
+
config HID_SMARTJOYPLUS
tristate "SmartJoy PLUS PS2/USB adapter support"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 23e6e3dd0c56..9f3fd2c21837 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtekff.o
obj-$(CONFIG_HID_HYPERV_MOUSE) += hid-hyperv.o
+obj-$(CONFIG_HID_HYPERX_HEADSET) += hid-hyperx-headset.o
obj-$(CONFIG_HID_ICADE) += hid-icade.o
obj-$(CONFIG_HID_ITE) += hid-ite.o
obj-$(CONFIG_HID_JABRA) += hid-jabra.o
diff --git a/drivers/hid/hid-hyperx-headset.c b/drivers/hid/hid-hyperx-headset.c
new file mode 100644
index 000000000000..b77931d4dfb0
--- /dev/null
+++ b/drivers/hid/hid-hyperx-headset.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HID driver for HyperX headsets
+ *
+ * Supports HyperX Cloud III Wireless headsets.
+ *
+ * Copyright (c) 2026 Sofia Schneider
+ */
+
+#include <linux/usb.h>
+#include <linux/hid.h>
+
+#include "hid-ids.h"
+
+#define HYPERX_POLL_INTERVAL_MS (2 * 60 * 1000)
+
+#define HYPERX_REPORT_ID 0x66
+#define HYPERX_PACKET_SIZE 62
+
+#define HYPERX_CMD_GET_CONNECTED 0x82
+#define HYPERX_CMD_GET_BATTERY 0x89
+#define HYPERX_CMD_GET_CHARGING 0x8A
+
+#define HYPERX_RESP_CONNECTED 0x0B
+#define HYPERX_RESP_CHARGING 0x0C
+#define HYPERX_RESP_BATTERY 0x0D
+
+#define HYPERX_PREFIX "HP, Inc "
+#define HYPERX_PREFIX_LEN strlen(HYPERX_PREFIX)
+
+struct hyperx_headset_device {
+ struct hid_device *hdev;
+ struct power_supply *battery;
+ struct power_supply_desc battery_desc;
+
+ spinlock_t lock;
+ u8 battery_level;
+ bool is_charging;
+ bool is_connected;
+
+ struct delayed_work poll_work;
+ struct work_struct battery_work;
+};
+
+static const enum power_supply_property hyperx_headset_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT, POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_SCOPE, POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static int hyperx_headset_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct hyperx_headset_device *drvdata = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = drvdata->is_connected ? 1 : 0;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = drvdata->battery_level;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ if (!drvdata->is_connected)
+ val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+ else if (drvdata->is_charging)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else if (drvdata->battery_level == 100)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = drvdata->hdev->name;
+ while (!strncmp(val->strval, HYPERX_PREFIX, HYPERX_PREFIX_LEN))
+ val->strval += HYPERX_PREFIX_LEN;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = "HyperX";
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+ return ret;
+}
+
+
+
+static int hyperx_headset_send_command(struct hyperx_headset_device *drvdata,
+ u8 command)
+{
+ struct hid_device *hdev = drvdata->hdev;
+ u8 *buf;
+ int ret;
+
+ buf = kzalloc(HYPERX_PACKET_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = HYPERX_REPORT_ID;
+ buf[1] = command;
+
+ ret = hid_hw_raw_request(hdev, HYPERX_REPORT_ID, buf,
+ HYPERX_PACKET_SIZE, HID_OUTPUT_REPORT,
+ HID_REQ_SET_REPORT);
+
+ if (ret < 0)
+ hid_err(hdev, "hw_raw_request failed (command 0x%02x)\n",
+ command);
+
+ kfree(buf);
+ return ret;
+}
+
+static void hyperx_headset_poll_work(struct work_struct *work)
+{
+ struct hyperx_headset_device *drvdata = container_of(
+ work, struct hyperx_headset_device, poll_work.work);
+
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_CONNECTED);
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_BATTERY);
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_CHARGING);
+
+ schedule_delayed_work(&drvdata->poll_work,
+ msecs_to_jiffies(HYPERX_POLL_INTERVAL_MS));
+}
+
+static void hyperx_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static void hyperx_headset_battery_work(struct work_struct *work)
+{
+ struct hyperx_headset_device *drvdata =
+ container_of(work, struct hyperx_headset_device, battery_work);
+ struct power_supply_config battery_cfg = { .drv_data = drvdata };
+ unsigned long flags;
+ bool connected;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ connected = drvdata->is_connected;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ hyperx_headset_set_wireless_status(drvdata->hdev, connected);
+
+ if (connected && !drvdata->battery) {
+ struct power_supply *ps;
+
+ ps = power_supply_register(&drvdata->hdev->dev,
+ &drvdata->battery_desc,
+ &battery_cfg);
+ if (IS_ERR(ps)) {
+ hid_err(drvdata->hdev,
+ "power_supply_register failed\n");
+ return;
+ }
+
+ power_supply_powers(ps, &drvdata->hdev->dev);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ drvdata->battery = ps;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+ } else if (!connected && drvdata->battery) {
+ struct power_supply *ps;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ ps = drvdata->battery;
+ drvdata->battery = NULL;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ power_supply_unregister(ps);
+ }
+}
+
+static void
+hyperx_headset_parse_battery_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ u8 state1 = data[2];
+ u8 state2 = data[3];
+ u8 level = data[4];
+
+ // Battery event is invalid if both states are 0
+ if (state1 == 0 && state2 == 0)
+ return;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->battery_level != level) {
+ drvdata->battery_level = level;
+
+ if (drvdata->battery)
+ power_supply_changed(drvdata->battery);
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+}
+
+static void
+hyperx_headset_parse_charging_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ bool charging = (data[2] == 1);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->is_charging != charging) {
+ drvdata->is_charging = charging;
+
+ if (drvdata->battery)
+ power_supply_changed(drvdata->battery);
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+}
+
+static void
+hyperx_headset_parse_connected_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ bool state_changed = false;
+ bool connected = (data[2] == 1);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->is_connected != connected) {
+ drvdata->is_connected = connected;
+ state_changed = true;
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ if (state_changed)
+ schedule_work(&drvdata->battery_work);
+}
+
+static int hyperx_headset_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int ret;
+ struct hyperx_headset_device *drvdata;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (drvdata == NULL)
+ return -ENOMEM;
+ drvdata->hdev = hdev;
+ drvdata->is_connected = false;
+ drvdata->is_charging = false;
+ drvdata->battery_level = 100;
+ spin_lock_init(&drvdata->lock);
+ hid_set_drvdata(hdev, drvdata);
+
+ drvdata->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ drvdata->battery_desc.properties = hyperx_headset_battery_props;
+ drvdata->battery_desc.num_properties = ARRAY_SIZE(hyperx_headset_battery_props);
+ drvdata->battery_desc.get_property = hyperx_headset_battery_get_property;
+ drvdata->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+ "hyperx_headset_battery_%s",
+ strlen(hdev->uniq) ?
+ hdev->uniq : dev_name(&hdev->dev));
+ if (!drvdata->battery_desc.name)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&drvdata->poll_work, hyperx_headset_poll_work);
+ INIT_WORK(&drvdata->battery_work, hyperx_headset_battery_work);
+
+ ret = hid_parse(hdev);
+ if (ret != 0) {
+ hid_err(hdev, "parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret != 0) {
+ hid_err(hdev, "hw_start failed\n");
+ cancel_work_sync(&drvdata->battery_work);
+ return ret;
+ }
+
+ schedule_delayed_work(&drvdata->poll_work, 0);
+
+ return 0;
+}
+
+static int hyperx_headset_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct hyperx_headset_device *drvdata = hid_get_drvdata(hdev);
+
+ if (size < 5 || data[0] != HYPERX_REPORT_ID)
+ return 0;
+
+ switch (data[1]) {
+ case HYPERX_CMD_GET_CONNECTED:
+ case HYPERX_RESP_CONNECTED:
+ hyperx_headset_parse_connected_event(drvdata, data);
+ break;
+
+ case HYPERX_CMD_GET_BATTERY:
+ case HYPERX_RESP_BATTERY:
+ hyperx_headset_parse_battery_event(drvdata, data);
+ break;
+
+ case HYPERX_CMD_GET_CHARGING:
+ case HYPERX_RESP_CHARGING:
+ hyperx_headset_parse_charging_event(drvdata, data);
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static void hyperx_headset_remove(struct hid_device *hdev)
+{
+ struct hyperx_headset_device *drvdata = hid_get_drvdata(hdev);
+ struct power_supply *ps = NULL;
+ unsigned long flags;
+
+ hid_hw_stop(hdev);
+
+ if (drvdata) {
+ cancel_delayed_work_sync(&drvdata->poll_work);
+ cancel_work_sync(&drvdata->battery_work);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ ps = drvdata->battery;
+ drvdata->battery = NULL;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ if (ps)
+ power_supply_unregister(ps);
+ }
+}
+
+
+static const struct hid_device_id hyperx_headset_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_HP,
+ USB_DEVICE_ID_HP_HYPERX_CLOUD_III_WIRELESS) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, hyperx_headset_devices);
+
+static struct hid_driver hyperx_headset_driver = {
+ .name = "hyperx-headset",
+ .id_table = hyperx_headset_devices,
+ .probe = hyperx_headset_probe,
+ .raw_event = hyperx_headset_raw_event,
+ .remove = hyperx_headset_remove,
+};
+module_hid_driver(hyperx_headset_driver);
+
+MODULE_AUTHOR("Sofia Schneider <sofia@schn.dev>");
+MODULE_DESCRIPTION("HID driver for HyperX headsets");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1059922baaac..aa2c3a71315b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -696,6 +696,7 @@
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0941 0x0941
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0641 0x0641
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_1f4a 0x1f4a
+#define USB_DEVICE_ID_HP_HYPERX_CLOUD_III_WIRELESS 0x05b7
#define USB_VENDOR_ID_HUION 0x256c
#define USB_DEVICE_ID_HUION_TABLET 0x006e
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] HID: hyperx-headset: Add support for HyperX headset devices
From: sashiko-bot @ 2026-06-27 20:48 UTC (permalink / raw)
To: Sofia Schneider; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260627203948.1053589-1-sofia@schn.dev>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing power supply unregister on the `hid_hw_start` failure path causes a resource leak and Use-After-Free.
--
commit 100439ac6279809f99a9f4158c6fe4a272508223
Author: Sofia Schneider <sofia@schn.dev>
HID: hyperx-headset: Add support for HyperX headset devices
Introduce a HID driver for HyperX Cloud III Wireless headsets,
supporting battery reporting and connection status.
> diff --git a/drivers/hid/hid-hyperx-headset.c b/drivers/hid/hid-hyperx-headset.c
> new file mode 100644
> index 0000000000000..b77931d4dfb08
> --- /dev/null
> +++ b/drivers/hid/hid-hyperx-headset.c
[ ... ]
> +static int hyperx_headset_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
[ ... ]
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret != 0) {
> + hid_err(hdev, "hw_start failed\n");
> + cancel_work_sync(&drvdata->battery_work);
[Severity: High]
Does this error path leak the power supply if it was already registered?
If the device sends a connected event early during hid_hw_start(),
battery_work may execute concurrently. It can register the power supply
and store it in drvdata->battery. If hid_hw_start() subsequently fails
(e.g. during hid_connect), this error path cancels the work but fails
to unregister drvdata->battery.
Because drvdata is managed by devm, it will be freed when probe returns
an error. This leaves the registered power supply with a dangling pointer
to drvdata, which can lead to a use-after-free if its sysfs attributes
are accessed.
Should this path check and unregister drvdata->battery before returning?
> + return ret;
> + }
> +
> + schedule_delayed_work(&drvdata->poll_work, 0);
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627203948.1053589-1-sofia@schn.dev?part=1
^ permalink raw reply
* [PATCH v3] HID: hyperx-headset: Add support for HyperX headset devices
From: Sofia Schneider @ 2026-06-27 21:01 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, Sofia Schneider
Introduce a HID driver for HyperX Cloud III Wireless headsets,
supporting battery reporting and connection status.
Tested with a HyperX Cloud III Wireless only, for lack of
other testable devices.
Signed-off-by: Sofia Schneider <sofia@schn.dev>
---
v2 -> v3:
- Unregister power supply on hid_hw_start() failure in probe() to prevent UAF/resource leak.
v1 -> v2:
- Call hid_hw_stop() before cancelling work queues in remove() to prevent UAF.
- Safe unregistration of power supply using the spinlock in remove() to prevent race conditions.
- Cancel battery_work in the hid_hw_start() error path in probe() to prevent UAF.
- Dynamically allocate power supply names to avoid sysfs naming collisions.
v1: https://lore.kernel.org/linux-input/20260622022800.D01D51F000E9@smtp.kernel.org/T/
v2: https://lore.kernel.org/linux-input/20260627204840.2DD671F000E9@smtp.kernel.org/T/
MAINTAINERS | 6 +
drivers/hid/Kconfig | 11 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-hyperx-headset.c | 399 +++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
5 files changed, 418 insertions(+)
create mode 100644 drivers/hid/hid-hyperx-headset.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d8252026bbd4..fa49655255f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11419,6 +11419,12 @@ F: include/uapi/linux/hid*
F: samples/hid/
F: tools/testing/selftests/hid/
+HID HYPERX HEADSET DRIVER
+M: Sofia Schneider <sofia@schn.dev>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-hyperx-headset.c
+
HID LOGITECH DRIVERS
R: Filipe Laíns <lains@riseup.net>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index f9bcaeb66385..e9f5f1f982c9 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1215,6 +1215,17 @@ config HID_HYPERV_MOUSE
help
Select this option to enable the Hyper-V mouse driver.
+config HID_HYPERX_HEADSET
+ tristate "HyperX headset devices"
+ depends on USB_HID
+ select POWER_SUPPLY
+ help
+ Support for HyperX headset devices.
+
+ Say Y here if you would like to enable support for HyperX headset devices.
+ To compile this driver as a module, choose M here: the module will be called
+ hid-hyperx-headset.
+
config HID_SMARTJOYPLUS
tristate "SmartJoy PLUS PS2/USB adapter support"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 23e6e3dd0c56..9f3fd2c21837 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtekff.o
obj-$(CONFIG_HID_HYPERV_MOUSE) += hid-hyperv.o
+obj-$(CONFIG_HID_HYPERX_HEADSET) += hid-hyperx-headset.o
obj-$(CONFIG_HID_ICADE) += hid-icade.o
obj-$(CONFIG_HID_ITE) += hid-ite.o
obj-$(CONFIG_HID_JABRA) += hid-jabra.o
diff --git a/drivers/hid/hid-hyperx-headset.c b/drivers/hid/hid-hyperx-headset.c
new file mode 100644
index 000000000000..1564432a225c
--- /dev/null
+++ b/drivers/hid/hid-hyperx-headset.c
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HID driver for HyperX headsets
+ *
+ * Supports HyperX Cloud III Wireless headsets.
+ *
+ * Copyright (c) 2026 Sofia Schneider
+ */
+
+#include <linux/usb.h>
+#include <linux/hid.h>
+
+#include "hid-ids.h"
+
+#define HYPERX_POLL_INTERVAL_MS (2 * 60 * 1000)
+
+#define HYPERX_REPORT_ID 0x66
+#define HYPERX_PACKET_SIZE 62
+
+#define HYPERX_CMD_GET_CONNECTED 0x82
+#define HYPERX_CMD_GET_BATTERY 0x89
+#define HYPERX_CMD_GET_CHARGING 0x8A
+
+#define HYPERX_RESP_CONNECTED 0x0B
+#define HYPERX_RESP_CHARGING 0x0C
+#define HYPERX_RESP_BATTERY 0x0D
+
+#define HYPERX_PREFIX "HP, Inc "
+#define HYPERX_PREFIX_LEN strlen(HYPERX_PREFIX)
+
+struct hyperx_headset_device {
+ struct hid_device *hdev;
+ struct power_supply *battery;
+ struct power_supply_desc battery_desc;
+
+ spinlock_t lock;
+ u8 battery_level;
+ bool is_charging;
+ bool is_connected;
+
+ struct delayed_work poll_work;
+ struct work_struct battery_work;
+};
+
+static const enum power_supply_property hyperx_headset_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT, POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_SCOPE, POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static int hyperx_headset_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct hyperx_headset_device *drvdata = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = drvdata->is_connected ? 1 : 0;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = drvdata->battery_level;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ if (!drvdata->is_connected)
+ val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+ else if (drvdata->is_charging)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else if (drvdata->battery_level == 100)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = drvdata->hdev->name;
+ while (!strncmp(val->strval, HYPERX_PREFIX, HYPERX_PREFIX_LEN))
+ val->strval += HYPERX_PREFIX_LEN;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = "HyperX";
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+ return ret;
+}
+
+
+
+static int hyperx_headset_send_command(struct hyperx_headset_device *drvdata,
+ u8 command)
+{
+ struct hid_device *hdev = drvdata->hdev;
+ u8 *buf;
+ int ret;
+
+ buf = kzalloc(HYPERX_PACKET_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = HYPERX_REPORT_ID;
+ buf[1] = command;
+
+ ret = hid_hw_raw_request(hdev, HYPERX_REPORT_ID, buf,
+ HYPERX_PACKET_SIZE, HID_OUTPUT_REPORT,
+ HID_REQ_SET_REPORT);
+
+ if (ret < 0)
+ hid_err(hdev, "hw_raw_request failed (command 0x%02x)\n",
+ command);
+
+ kfree(buf);
+ return ret;
+}
+
+static void hyperx_headset_poll_work(struct work_struct *work)
+{
+ struct hyperx_headset_device *drvdata = container_of(
+ work, struct hyperx_headset_device, poll_work.work);
+
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_CONNECTED);
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_BATTERY);
+ hyperx_headset_send_command(drvdata, HYPERX_CMD_GET_CHARGING);
+
+ schedule_delayed_work(&drvdata->poll_work,
+ msecs_to_jiffies(HYPERX_POLL_INTERVAL_MS));
+}
+
+static void hyperx_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static void hyperx_headset_battery_work(struct work_struct *work)
+{
+ struct hyperx_headset_device *drvdata =
+ container_of(work, struct hyperx_headset_device, battery_work);
+ struct power_supply_config battery_cfg = { .drv_data = drvdata };
+ unsigned long flags;
+ bool connected;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ connected = drvdata->is_connected;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ hyperx_headset_set_wireless_status(drvdata->hdev, connected);
+
+ if (connected && !drvdata->battery) {
+ struct power_supply *ps;
+
+ ps = power_supply_register(&drvdata->hdev->dev,
+ &drvdata->battery_desc,
+ &battery_cfg);
+ if (IS_ERR(ps)) {
+ hid_err(drvdata->hdev,
+ "power_supply_register failed\n");
+ return;
+ }
+
+ power_supply_powers(ps, &drvdata->hdev->dev);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ drvdata->battery = ps;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+ } else if (!connected && drvdata->battery) {
+ struct power_supply *ps;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ ps = drvdata->battery;
+ drvdata->battery = NULL;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ power_supply_unregister(ps);
+ }
+}
+
+static void
+hyperx_headset_parse_battery_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ u8 state1 = data[2];
+ u8 state2 = data[3];
+ u8 level = data[4];
+
+ // Battery event is invalid if both states are 0
+ if (state1 == 0 && state2 == 0)
+ return;
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->battery_level != level) {
+ drvdata->battery_level = level;
+
+ if (drvdata->battery)
+ power_supply_changed(drvdata->battery);
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+}
+
+static void
+hyperx_headset_parse_charging_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ bool charging = (data[2] == 1);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->is_charging != charging) {
+ drvdata->is_charging = charging;
+
+ if (drvdata->battery)
+ power_supply_changed(drvdata->battery);
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+}
+
+static void
+hyperx_headset_parse_connected_event(struct hyperx_headset_device *drvdata,
+ u8 *data)
+{
+ unsigned long flags;
+ bool state_changed = false;
+ bool connected = (data[2] == 1);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+
+ if (drvdata->is_connected != connected) {
+ drvdata->is_connected = connected;
+ state_changed = true;
+ }
+
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ if (state_changed)
+ schedule_work(&drvdata->battery_work);
+}
+
+static int hyperx_headset_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int ret;
+ struct hyperx_headset_device *drvdata;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (drvdata == NULL)
+ return -ENOMEM;
+ drvdata->hdev = hdev;
+ drvdata->is_connected = false;
+ drvdata->is_charging = false;
+ drvdata->battery_level = 100;
+ spin_lock_init(&drvdata->lock);
+ hid_set_drvdata(hdev, drvdata);
+
+ drvdata->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ drvdata->battery_desc.properties = hyperx_headset_battery_props;
+ drvdata->battery_desc.num_properties = ARRAY_SIZE(hyperx_headset_battery_props);
+ drvdata->battery_desc.get_property = hyperx_headset_battery_get_property;
+ drvdata->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+ "hyperx_headset_battery_%s",
+ strlen(hdev->uniq) ?
+ hdev->uniq : dev_name(&hdev->dev));
+ if (!drvdata->battery_desc.name)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&drvdata->poll_work, hyperx_headset_poll_work);
+ INIT_WORK(&drvdata->battery_work, hyperx_headset_battery_work);
+
+ ret = hid_parse(hdev);
+ if (ret != 0) {
+ hid_err(hdev, "parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret != 0) {
+ struct power_supply *ps;
+ unsigned long flags;
+
+ hid_err(hdev, "hw_start failed\n");
+ cancel_work_sync(&drvdata->battery_work);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ ps = drvdata->battery;
+ drvdata->battery = NULL;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ if (ps)
+ power_supply_unregister(ps);
+
+ return ret;
+ }
+
+ schedule_delayed_work(&drvdata->poll_work, 0);
+
+ return 0;
+}
+
+static int hyperx_headset_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct hyperx_headset_device *drvdata = hid_get_drvdata(hdev);
+
+ if (size < 5 || data[0] != HYPERX_REPORT_ID)
+ return 0;
+
+ switch (data[1]) {
+ case HYPERX_CMD_GET_CONNECTED:
+ case HYPERX_RESP_CONNECTED:
+ hyperx_headset_parse_connected_event(drvdata, data);
+ break;
+
+ case HYPERX_CMD_GET_BATTERY:
+ case HYPERX_RESP_BATTERY:
+ hyperx_headset_parse_battery_event(drvdata, data);
+ break;
+
+ case HYPERX_CMD_GET_CHARGING:
+ case HYPERX_RESP_CHARGING:
+ hyperx_headset_parse_charging_event(drvdata, data);
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static void hyperx_headset_remove(struct hid_device *hdev)
+{
+ struct hyperx_headset_device *drvdata = hid_get_drvdata(hdev);
+ struct power_supply *ps = NULL;
+ unsigned long flags;
+
+ hid_hw_stop(hdev);
+
+ if (drvdata) {
+ cancel_delayed_work_sync(&drvdata->poll_work);
+ cancel_work_sync(&drvdata->battery_work);
+
+ spin_lock_irqsave(&drvdata->lock, flags);
+ ps = drvdata->battery;
+ drvdata->battery = NULL;
+ spin_unlock_irqrestore(&drvdata->lock, flags);
+
+ if (ps)
+ power_supply_unregister(ps);
+ }
+}
+
+
+static const struct hid_device_id hyperx_headset_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_HP,
+ USB_DEVICE_ID_HP_HYPERX_CLOUD_III_WIRELESS) },
+ {}
+};
+MODULE_DEVICE_TABLE(hid, hyperx_headset_devices);
+
+static struct hid_driver hyperx_headset_driver = {
+ .name = "hyperx-headset",
+ .id_table = hyperx_headset_devices,
+ .probe = hyperx_headset_probe,
+ .raw_event = hyperx_headset_raw_event,
+ .remove = hyperx_headset_remove,
+};
+module_hid_driver(hyperx_headset_driver);
+
+MODULE_AUTHOR("Sofia Schneider <sofia@schn.dev>");
+MODULE_DESCRIPTION("HID driver for HyperX headsets");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1059922baaac..aa2c3a71315b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -696,6 +696,7 @@
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0941 0x0941
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0641 0x0641
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_1f4a 0x1f4a
+#define USB_DEVICE_ID_HP_HYPERX_CLOUD_III_WIRELESS 0x05b7
#define USB_VENDOR_ID_HUION 0x256c
#define USB_DEVICE_ID_HUION_TABLET 0x006e
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] Input: synaptics-rmi4 - bound the F54 report size to the allocated buffer
From: Bryam Vargas @ 2026-06-27 21:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Guenter Roeck, Benjamin Tissoires
In-Reply-To: <aj4LoEHHCMqUnyy2@google.com>
Hi Dmitry,
Thanks for picking this up and keeping the authorship on 03.
> there are more changes needed in F54. I incorporated it in the series I
> just posted, would appreviate if you could review it.
Went through all ten. It holds together. A few things worth flagging:
Backport ordering is the one to act on. 03 leans on 02's control flow: its
report_size > max_report_size guard lands in the worker tail that 02 folds
into the single out: write. And 03's Fixes: c762cc68b6a1 (Nov 2016) postdates
02's Fixes: 3a762dbd5347 (Jul 2016), so any stable tree old enough to want 03
still carries the abort:/error: form 02 reworks; cherry-picking 03 alone
conflicts on that hunk, and 06 touches the same tail. Keeping 02 -> 03 -> 06
together for stable (or a dependency note on 03) keeps AUTOSEL from taking 03
on its own.
01 is correct but instance-complete: a device honestly reporting a large F55
num_tx still oversizes report_size until 03 lands. Maybe a line in its commit
log pointing at 03.
One aside, not for this series: F54_FIFO_OFFSET is 16-bit but report_size
reaches 2 * 255 * 255 = 130050, so past 65535 the start offset wraps and the
device reads from the wrong place. It stays in bounds (report_data + i tracks
the same product), so it's correctness, not safety; maybe a later guard.
Otherwise it's straightforward. For the series:
Reviewed-by: Bryam Vargas <hexlabsecurity@proton.me>
03 is the one I sent, so drop it there. On the memory-safety patches I ran an
in-kernel A/B under KASAN: the unpatched arm overruns the plane (07), delivers
the stale frame (06), or overruns report_data (03's class); the patched arm is
clean. So for 05, 06 and 07:
Tested-by: Bryam Vargas <hexlabsecurity@proton.me>
Thanks,
Bryam
^ permalink raw reply
* [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
From: Wahid7852 @ 2026-06-27 22:47 UTC (permalink / raw)
To: linux-input; +Cc: srinivas.pandruvada, jikos, Wahid7852
Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
The current guard collapses two distinct cases into one condition:
if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
This incorrectly treats a valid post-init firmware announcement as a
corrupted packet, triggering an ISH soft-reset and a full ISHTP
re-initialisation cycle on each occurrence (~1100 times per day on
affected hardware).
Split the check: reject messages with no response flag as before, but
simply discard valid ENUM_DEVICES messages that arrive after init is
complete.
Signed-off-by: Wahid <wahidzk0091@gmail.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 6d64008f2..ec5e8afe7 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
switch (recv_msg->hdr.command & CMD_MASK) {
case HOSTIF_DM_ENUM_DEVICES:
- if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
- client_data->init_done)) {
+ if (!(recv_msg->hdr.command & ~CMD_MASK)) {
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
@@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
+ if (client_data->init_done)
+ break;
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
From: sashiko-bot @ 2026-06-27 22:57 UTC (permalink / raw)
To: Wahid7852; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260627224753.35075-1-wahidzk0091@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Duplicate firmware responses during initialization (ENUM_DEVICES, GET_HID_DESCRIPTOR, GET_REPORT_DESCRIPTOR) cause heap buffer overflows.
- [High] Missing payload length validation and flawed bounds checks in HOSTIF_DM_ENUM_DEVICES parsing lead to out-of-bounds memory reads.
- [High] Unscaled pointer arithmetic and integer underflow in HOSTIF_PUBLISH_INPUT_REPORT_LIST lead to massive out-of-bounds memory accesses.
--
commit 44b4c0eea0f41216fd9e39bd67a4356eeadc37c7
Author: Wahid <wahidzk0091@gmail.com>
HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
This commit splits the guard condition for HOSTIF_DM_ENUM_DEVICES messages
to safely discard valid ENUM_DEVICES messages that arrive after
initialization is complete. This prevents unnecessary ISH soft-resets
caused by periodic firmware announcements.
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl, recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned int)*payload;
[Severity: High]
This is a pre-existing issue, but does this code read out-of-bounds memory
if the payload length is zero?
If a HOSTIF_DM_ENUM_DEVICES message is received with a zero-length payload,
*payload is dereferenced here without validation.
Furthermore, in the loop that follows:
process_recv() {
...
for (i = 0; i < client_data->hid_dev_count; ++i) {
...
if (1 + sizeof(struct device_info) * i >= data_len)
break;
dev_info = (struct device_info *)(payload + 1 +
sizeof(struct device_info) * i);
if (client_data->hid_devices)
memcpy(client_data->hid_devices + i, dev_info,
sizeof(struct device_info));
...
}
The bounds check compares the relative payload offset against the absolute
buffer size (data_len) instead of the payload size. This allows the offset
to read out-of-bounds memory, which is then copied into the persistent
device information cache, leaking the contents.
> if (!client_data->hid_devices)
> client_data->hid_devices = devm_kcalloc(
[Severity: Critical]
This is a pre-existing issue, but can duplicate firmware responses cause a
heap buffer overflow here?
If multiple HOSTIF_DM_ENUM_DEVICES or GET_HID_DESCRIPTOR messages arrive
back-to-back before init_done becomes true, this allocation is incorrectly
skipped because the pointer is already non-NULL.
The memory is then copied into the existing buffer based on a potentially
larger, unvalidated hid_dev_count or payload_len:
process_recv() {
...
memcpy(client_data->hid_devices + i, dev_info,
sizeof(struct device_info));
...
memcpy(client_data->hid_descr[curr_hid_dev], payload, payload_len);
...
}
This could cause memory to be copied into the original buffer without
reallocation, causing a heap buffer overflow.
[ ... ]
[Severity: High]
This is a pre-existing issue, but is there an integer underflow and pointer
arithmetic issue when parsing HOSTIF_PUBLISH_INPUT_REPORT_LIST?
Looking further down in process_recv():
process_recv() {
...
case HOSTIF_PUBLISH_INPUT_REPORT_LIST:
...
payload_len = report_len - sizeof(struct hostif_msg_hdr);
...
report += sizeof(*report) + payload_len;
...
}
If the firmware-controlled report_len is less than 6
(sizeof(struct hostif_msg_hdr)), payload_len underflows.
Additionally, because report is a typed pointer (struct report *), this
addition is implicitly scaled by sizeof(struct report) (8 bytes), advancing
the pointer out of bounds by a factor of 8.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627224753.35075-1-wahidzk0091@gmail.com?part=1
^ permalink raw reply
* [PATCH] HID: steam: reject short serial number reports
From: Yousef Alhouseen @ 2026-06-28 0:41 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Vicki Pfau, stable,
syzbot+75f3f9bff8c510602d36, Yousef Alhouseen
steam_recv_report() may return a short positive response and copies
only the bytes actually received. steam_get_serial() nevertheless reads
the full three-byte header and trusts its length without checking that
the serial payload was returned.
A malformed USB device can therefore make the driver read uninitialized
stack bytes. With a complete-looking short header, those bytes can also
be copied into steam->serial_no and printed.
Account for the stripped report ID in the return value and reject replies
that do not contain both the header and its declared payload.
Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-steam.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 197126d6e081..8c8bfb10e8b8 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -454,11 +454,20 @@ static int steam_get_serial(struct steam_device *steam)
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
goto out;
+ /* hid_hw_raw_request() counts the stripped report ID byte. */
+ if (ret < 4) {
+ ret = -EIO;
+ goto out;
+ }
if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
ret = -EIO;
goto out;
}
+ if (ret - 1 < 3 + reply[1]) {
+ ret = -EIO;
+ goto out;
+ }
reply[3 + STEAM_SERIAL_LEN] = 0;
strscpy(steam->serial_no, reply + 3, reply[1]);
out:
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] HID: steam: reject short serial number reports
From: Vicki Pfau @ 2026-06-28 0:47 UTC (permalink / raw)
To: Yousef Alhouseen, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, syzbot+75f3f9bff8c510602d36
In-Reply-To: <20260628004106.26920-1-alhouseenyousef@gmail.com>
Hi Yousef,
On 6/27/26 5:41 PM, Yousef Alhouseen wrote:
> steam_recv_report() may return a short positive response and copies
> only the bytes actually received. steam_get_serial() nevertheless reads
> the full three-byte header and trusts its length without checking that
> the serial payload was returned.
>
> A malformed USB device can therefore make the driver read uninitialized
> stack bytes. With a complete-looking short header, those bytes can also
> be copied into steam->serial_no and printed.
>
> Account for the stripped report ID in the return value and reject replies
> that do not contain both the header and its declared payload.
>
> Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
> Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> Cc: stable@vger.kernel.org
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> drivers/hid/hid-steam.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 197126d6e081..8c8bfb10e8b8 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -454,11 +454,20 @@ static int steam_get_serial(struct steam_device *steam)
> ret = steam_recv_report(steam, reply, sizeof(reply));
> if (ret < 0)
> goto out;
> + /* hid_hw_raw_request() counts the stripped report ID byte. */
> + if (ret < 4) {
> + ret = -EIO;
> + goto out;
> + }
> if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> ret = -EIO;
> goto out;
> }
> + if (ret - 1 < 3 + reply[1]) {
> + ret = -EIO;
> + goto out;
> + }
> reply[3 + STEAM_SERIAL_LEN] = 0;
> strscpy(steam->serial_no, reply + 3, reply[1]);
> out:
I already have locally a patch that fixes this as part of my pending Steam Controller 2 support. However, it chooses to fix it in a different way that would affect all uses of steam_recv_report instead of per-callsite (with only one callsite fixed). I am hoping to get this patchset submitted soon, once more widescale testing is done, but if you want in the meantime I can pull out that single fix and submit it separately; it's a bit more sprawling and involves adding a new function for combined send/recv.
Vicki
^ permalink raw reply
* Re: [PATCH] HID: steam: reject short serial number reports
From: sashiko-bot @ 2026-06-28 0:54 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628004106.26920-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The serial number payload is systematically truncated and its buffer is improperly NUL-terminated. `strscpy(steam->serial_no, reply + 3, reply[1])` incorrectly uses the dynamic payload length (`reply[1]`) as the destination buffer size limit. Because `strscpy` guarantees NUL-termination by copying at most `size - 1` bytes, it unconditionally truncates the last byte of the serial number. Furthermore, `reply[3 + STEAM_SERIAL_LEN] = 0` hardcodes the NUL terminator at index 24 (the maximum possible payload boundary). If the device returns the maximum 22-byte payload, this overwrites the 22nd payload byte before copying. If the payload is shorter, it leaves uninitialized stack memory between the end of the payload and the NUL terminator, which would be exposed if `strscpy` used the correct buffer size.
- [High] Works and delayed works are cancelled before the underlying hardware is stopped, creating a race condition that leads to a Use-After-Free (UAF) on the `steam_device` structure.
--
commit 9b12928f5d4ac893e4176a52f291e99aaa7d63e9
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: steam: reject short serial number reports
This patch modifies the steam driver to correctly validate the length of
the serial number response. It accounts for the stripped report ID in the
return value and ensures that both the header and its declared payload are
fully received before parsing.
Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 197126d6e0810..8c8bfb10e8b82 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -454,11 +454,20 @@ static int steam_get_serial(struct steam_device *steam)
> ret = steam_recv_report(steam, reply, sizeof(reply));
> if (ret < 0)
> goto out;
> + /* hid_hw_raw_request() counts the stripped report ID byte. */
> + if (ret < 4) {
> + ret = -EIO;
> + goto out;
> + }
> if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> ret = -EIO;
> goto out;
> }
> + if (ret - 1 < 3 + reply[1]) {
> + ret = -EIO;
> + goto out;
> + }
> reply[3 + STEAM_SERIAL_LEN] = 0;
> strscpy(steam->serial_no, reply + 3, reply[1]);
[Severity: High]
This is a pre-existing issue, but does this logic truncate the serial number?
Because strscpy() guarantees NUL-termination by copying at most size - 1
bytes, passing reply[1] as the destination size limit will unconditionally
truncate the last byte of the serial number payload.
Additionally, setting reply[3 + STEAM_SERIAL_LEN] = 0 hardcodes the NUL
terminator at the maximum boundary.
If the device returns the maximum 22-byte payload, this overwrites the 22nd
payload byte. If the payload is shorter, could this leave uninitialized
stack memory between the end of the payload and the NUL terminator?
I also noticed another potential problem in steam_remove() which is not
touched by this patch:
> static void steam_remove(struct hid_device *hdev)
> {
[ ... ]
> cancel_delayed_work_sync(&steam->mode_switch);
> cancel_work_sync(&steam->work_connect);
> cancel_work_sync(&steam->rumble_work);
> cancel_work_sync(&steam->unregister_work);
> steam->client_hdev = NULL;
> steam->client_opened = 0;
> if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> hid_info(hdev, "Steam wireless receiver disconnected");
> }
> hid_hw_close(hdev);
> hid_hw_stop(hdev);
[Severity: High]
This isn't a bug introduced by this patch, but could this cancellation
sequence cause a use-after-free on the steam struct?
Since the driver explicitly cancels works before stopping the underlying
hardware with hid_hw_close() and hid_hw_stop(), an incoming interrupt could
trigger steam_raw_event() and reschedule the work.
If the work is rescheduled right before steam_remove() returns, could the
workqueue execute on the freed steam struct after the driver core cleans
it up?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628004106.26920-1-alhouseenyousef@gmail.com?part=1
^ 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