* [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
asus_kbd_register_leds(), I noticed it registers a listener to a global list
and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
fails the driver returns without unregistering the listener, and the devres
subsystem will automatically free the memory, leaving a dangling pointer
in the global list.
Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 0a6c97155549..f38b18ad65c6 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (drvdata->tp) {
ret = asus_start_multitouch(hdev);
if (ret)
- goto err_stop_hw;
+ goto err_unregister_backlight;
}
}
return 0;
+err_unregister_backlight:
+ if (drvdata->kbd_backlight) {
+ asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
+ devm_kfree(&hdev->dev, drvdata->kbd_backlight);
+ drvdata->kbd_backlight = NULL;
+ }
err_stop_hw:
hid_hw_stop(hdev);
return ret;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.
This commit changes when methods are called, not method parameters:
the driver behaves as before.
Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
1 file changed, 56 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f38b18ad65c6..a6467172c455 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -150,6 +150,13 @@ struct asus_drvdata {
unsigned long battery_next_query;
struct work_struct fn_lock_sync_work;
bool fn_lock;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ struct work_struct wmi_work;
+ bool wmi_work_disabled;
+ u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
+ int wmi_size;
+ spinlock_t wmi_lock;
+#endif
struct asus_xgm_led *xgm_led;
};
@@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
return 0;
}
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
/*
* Send events to asus-wmi driver for handling special keys
*/
@@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
return 0;
}
+static void asus_wmi_work(struct work_struct *work)
+{
+ struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
+ u8 report_data[FEATURE_KBD_REPORT_SIZE];
+ int report_size;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&drvdata->wmi_lock, flags);
+ memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
+ report_size = drvdata->wmi_size;
+ spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+ ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
+ if (ret) {
+ if (ret != -ENODEV)
+ hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
+
+ /* Fallback: pass the raw event to the HID core */
+ hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
+ report_data, report_size,
+ report_size, 1);
+ }
+}
+#endif
+
static int asus_event(struct hid_device *hdev, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
@@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ unsigned long flags;
+#endif
if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
return asus_report_battery(drvdata, data, size);
@@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
* pass to userspace so it can implement its own fan control.
*/
if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
- int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
-
- if (ret == 0) {
- /* Successfully handled by asus-wmi, block event */
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ spin_lock_irqsave(&drvdata->wmi_lock, flags);
+ memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
+ drvdata->wmi_size = size;
+ spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
+
+ if (!drvdata->wmi_work_disabled) {
+ schedule_work(&drvdata->wmi_work);
+ /* Successfully handled asynchronously, block event */
return -1;
}
-
- /*
- * Warn if asus-wmi failed (but not if it's unavailable).
- * Let the event reach userspace in all failure cases.
- */
- if (ret != -ENODEV)
- hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
+#endif
}
/*
@@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
drvdata->hdev = hdev;
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
+ spin_lock_init(&drvdata->wmi_lock);
+#endif
if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
ret = asus_battery_probe(hdev);
@@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
+#if IS_REACHABLE(CONFIG_ASUS_WMI)
+ drvdata->wmi_work_disabled = true;
+ cancel_work_sync(&drvdata->wmi_work);
+#endif
+
if (drvdata->xgm_led)
led_classdev_unregister(&drvdata->xgm_led->cdev);
--
2.47.3
^ permalink raw reply related
* [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli, sahiko-bot
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
schedule_work() after the work was supposedly canceled.
Once the driver is removed and drvdata is freed by devres, the workqueue
could execute asus_sync_fn_lock() and dereference the freed drvdata.
Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
Reported-by: sahiko-bot@kernel.org
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a6467172c455..a65a28137f0d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -148,6 +148,7 @@ struct asus_drvdata {
int battery_stat;
bool battery_in_query;
unsigned long battery_next_query;
+ bool fn_lock_sync_work_disabled;
struct work_struct fn_lock_sync_work;
bool fn_lock;
#if IS_REACHABLE(CONFIG_ASUS_WMI)
@@ -416,7 +417,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
case KEY_KBDILLUMTOGGLE:
return !asus_hid_event(ASUS_EV_BRTTOGGLE);
case KEY_FN_ESC:
- if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ if (!drvdata->fn_lock_sync_work_disabled && (drvdata->quirks & QUIRK_HID_FN_LOCK)) {
drvdata->fn_lock = !drvdata->fn_lock;
schedule_work(&drvdata->fn_lock_sync_work);
}
@@ -1497,8 +1498,10 @@ static void asus_remove(struct hid_device *hdev)
cancel_work_sync(&drvdata->kbd_backlight->work);
}
- if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+ if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ drvdata->fn_lock_sync_work_disabled = true;
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ }
#if IS_REACHABLE(CONFIG_ASUS_WMI)
drvdata->wmi_work_disabled = true;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 7/8] HID: asus: add i2c entry for FA808UM and other TUFs
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
On newer TUF laptops the keyboard HID device uses the same PID/VID of a
USB device that was found in ROG laptops: add it to hid-asus as i2c too.
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index a65a28137f0d..efad6b623e65 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1646,6 +1646,9 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
--
2.47.3
^ permalink raw reply related
* [PATCH v3 8/8] HID: asus: remove unnecessary OOM message
From: Denis Benato @ 2026-06-13 15:30 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato, Antheas Kapenekakis,
Connor Belli
In-Reply-To: <20260613153029.2559774-1-denis.benato@linux.dev>
If devm_kzalloc fails an allocation error is already being reported:
there is no need for the driver to repeat it, moreover if this were
new code checkpatch.pl would throw a warning.
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index efad6b623e65..f1fe3358a298 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1322,10 +1322,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
- if (drvdata == NULL) {
- hid_err(hdev, "Can't alloc Asus descriptor\n");
+ if (drvdata == NULL)
return -ENOMEM;
- }
hid_set_drvdata(hdev, drvdata);
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3 3/8] HID: asus: add support for xgm led
From: sashiko-bot @ 2026-06-13 15:42 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-4-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing led_classdev_unregister() in the probe error path leads to a use-after-free.
- [Medium] An error during keyboard backlight resume causes the XGM LED resume to be skipped.
- [Medium] Returning a positive short transfer length from brightness_set_blocking instead of a negative error code.
--
commit 201b953e7851d7aae6a4427d79499eefd3b7b433
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for xgm led
XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 52d755659d951..95e7dc6a447d8 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -941,6 +949,23 @@ static int asus_battery_probe(struct hid_device *hdev)
> return ret;
> }
>
> +static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
> +{
> + const u8 buf[ROG_XGM_REPORT_SIZE] = {
> + FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
> + };
> + struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
> + int ret;
> +
> + ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
> + if (ret != ROG_XGM_REPORT_SIZE) {
> + hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
> + return ret;
[Severity: Medium]
If a short transfer occurs, asus_kbd_set_report() will return a positive
number less than ROG_XGM_REPORT_SIZE.
Will returning this positive value cause the LED subsystem API to incorrectly
interpret the short transfer as a success instead of a failure?
> + }
> +
> + return 0;
> +}
> +
[ ... ]
> @@ -1178,6 +1248,14 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
> }
> }
>
[Severity: Medium]
If restoring the keyboard backlight fails earlier in asus_resume(), the
early return via goto asus_resume_err causes this XGM LED resume logic
to be skipped.
Should the device resume function try to resume as many independent
components as possible instead of unconditionally aborting here?
> + if (drvdata->xgm_led) {
> + ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
> + if (ret) {
> + hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
> + goto asus_resume_err;
> + }
> + }
> +
> asus_resume_err:
> return ret;
> }
[ ... ]
> @@ -1304,6 +1382,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
> }
>
> + if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> + ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> + (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> + ret = asus_xgm_init(hdev, drvdata);
[Severity: High]
If asus_probe() encounters an error later on (such as asus_start_multitouch()
failing), the error handling path does not unregister the LED class device.
Because drvdata->xgm_led was allocated with devm_kzalloc(), devres will
free it. Does this lead to a use-after-free if userspace interacts with the
sysfs interface of the leaked cdev structure?
Would using devm_led_classdev_register() in asus_xgm_init() instead of
led_classdev_register() cleanly avoid this issue?
> + if (ret) {
> + hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
> + goto err_stop_hw;
> + }
> + }
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=3
^ permalink raw reply
* Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: sashiko-bot @ 2026-06-13 15:42 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-5-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The newly added `err_unregister_backlight` error path frees `drvdata->kbd_backlight` without canceling its associated workqueue, leading to a Use-After-Free.
- [High] The `asus_probe` error path fails to unregister the dynamically registered `xgm_led` class device, leaving a dangling pointer in the LED subsystem.
Pre-existing issues:
- [High] The `asus_probe` error paths fail to cancel `fn_lock_sync_work` and `wmi_work`, creating a potential Use-After-Free if the works are scheduled during probe.
--
commit 6e04c2967d627dea02497ed7471439a5d35c56bc
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: cleanup keyboard listener on failure: avoid use-after-free
This commit fixes a potential use-after-free by unregistering the keyboard
listener on probe failure. It explicitly adds an error path to unregister
the listener and free the memory when a subsequent initialization step fails.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 95e7dc6a447d8..e3a03f7cf77d0 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1420,11 +1420,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (drvdata->tp) {
> ret = asus_start_multitouch(hdev);
> if (ret)
> - goto err_stop_hw;
> + goto err_unregister_backlight;
> }
> }
>
> return 0;
> +err_unregister_backlight:
> + if (drvdata->kbd_backlight) {
> + asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> + devm_kfree(&hdev->dev, drvdata->kbd_backlight);
[Severity: High]
Does this code leave pending work items referencing freed memory?
If concurrent events have queued `drvdata->kbd_backlight->work` (e.g.,
via `asus_kbd_backlight_set()` or `asus_hid_event()`), freeing
`kbd_backlight` without synchronizing via `cancel_work_sync()` might
cause a use-after-free when the workqueue executes.
> + drvdata->kbd_backlight = NULL;
> + }
> err_stop_hw:
> hid_hw_stop(hdev);
> return ret;
[Severity: High]
Could failing to unregister `xgm_led` leave a dangling pointer in the LED
subsystem?
If `asus_xgm_init()` succeeds, it registers `drvdata->xgm_led->cdev` using
`led_classdev_register()`, which is not devres-managed. If `asus_probe()`
subsequently fails and jumps here, the code exits without calling
`led_classdev_unregister()`. Since `drvdata->xgm_led` is devres-managed, it
will be freed, but the LED subsystem will retain a pointer to it,
potentially causing a use-after-free.
[Severity: High]
This is a pre-existing issue, but does this error path also need to cancel
`fn_lock_sync_work` and `wmi_work`?
Once the hardware is started via `hid_hw_start()`, incoming input events
can schedule `drvdata->fn_lock_sync_work` or `drvdata->wmi_work`. If
`asus_probe()` fails and exits here, devres will automatically free
`drvdata`. Since `cancel_work_sync()` is omitted from this error path, any
queued work could execute asynchronously after `drvdata` is freed, leading
to a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=4
^ permalink raw reply
* Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: sashiko-bot @ 2026-06-13 15:43 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-6-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unbounded wmi_size assignment causes a stack buffer overflow and out-of-bounds read in asus_wmi_work().
- [High] Asynchronous invocation of hid_report_raw_event without acquiring driver_input_lock causes a data race.
- [High] Missing cancel_work_sync in asus_probe error paths leaves wmi_work pending, causing a Use-After-Free.
- [Medium] Passing report_size as bufsize to hid_report_raw_event silently drops valid short reports.
--
commit 59c560d9da130f0e811e0abb99b0edf58eef0925
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: avoid sleeping calls in atomic context
Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
asus_raw_event() that is called in atomic context.
This commit changes when methods are called, not method parameters:
the driver behaves as before.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e3a03f7cf77d0..a3437789b2341 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
> return 0;
> }
>
> +static void asus_wmi_work(struct work_struct *work)
> +{
> + struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> + u8 report_data[FEATURE_KBD_REPORT_SIZE];
> + int report_size;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
[Severity: High]
Can this memcpy cause a stack buffer overflow?
In asus_raw_event(), drvdata->wmi_size is assigned the untrusted raw event
size without being clipped to the maximum buffer size. When the asynchronous
worker processes this, it uses that unclipped size as the length for memcpy
into the fixed 64-byte report_data stack array.
A malicious or malfunctioning USB device sending a report larger than
64 bytes could overflow the stack here.
> + report_size = drvdata->wmi_size;
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> + if (ret) {
> + if (ret != -ENODEV)
> + hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> + /* Fallback: pass the raw event to the HID core */
> + hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> + report_data, report_size,
> + report_size, 1);
[Severity: High]
Is it safe to call hid_report_raw_event() asynchronously without acquiring
hdev->driver_input_lock?
The HID core expects callers of hid_report_raw_event() to hold this lock
to ensure exclusive access when modifying shared state within the HID report
structure. If a concurrent hardware interrupt triggers __hid_input_report(),
it could successfully acquire the lock and mutate the HID report state at the
same time, potentially leading to data corruption or crashes.
[Severity: Medium]
Does passing report_size as the bufsize parameter prevent the HID core from
safely handling short reports?
The HID core requires bufsize to be at least the expected size of the report
so it can safely zero-pad short reports. Because bufsize is constrained here
to the actual payload size, the safety check inside hid_report_raw_event()
might fail, causing valid short reports to be permanently dropped with an
EINVAL error.
> + }
> +}
> +#endif
> +
[ ... ]
> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
> * pass to userspace so it can implement its own fan control.
> */
> if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
> - int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> -
> - if (ret == 0) {
> - /* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> + drvdata->wmi_size = size;
[Severity: High]
Should drvdata->wmi_size be capped to the size of the buffer?
While the memcpy directly above safely clips the copy using min_t(),
drvdata->wmi_size is assigned the unclipped size, leading to the stack buffer
overflow when it is consumed in asus_wmi_work().
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + if (!drvdata->wmi_work_disabled) {
> + schedule_work(&drvdata->wmi_work);
> + /* Successfully handled asynchronously, block event */
> return -1;
> }
[ ... ]
> @@ -1344,6 +1380,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
> drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> + spin_lock_init(&drvdata->wmi_lock);
> +#endif
[Severity: High]
Could wmi_work trigger a use-after-free if probe fails during later stages?
In asus_probe(), after wmi_work is initialized, hid_hw_start() is called which
enables hardware interrupts. At this point, incoming events can queue wmi_work.
If a subsequent initialization step fails (for example, asus_xgm_init()),
the function jumps to the err_stop_hw label:
err_stop_hw:
hid_hw_stop(hdev);
return ret;
This returns an error without calling cancel_work_sync(&drvdata->wmi_work).
The workqueue subsystem guidelines state that once a work struct is queued,
shutdown methods must prevent leaking it. The drvdata memory will be freed
by devres, and any pending wmi_work will execute on freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=5
^ permalink raw reply
* Re: [PATCH v3 7/8] HID: asus: add i2c entry for FA808UM and other TUFs
From: sashiko-bot @ 2026-06-13 15:45 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260613153029.2559774-8-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] NULL pointer dereference or UAF in asus_e1239t_event when handling a Medion E1239T device with no input usages
--
commit f6d872852d0965599a5f0e0ef39dc3d4ec54c196
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add i2c entry for FA808UM and other TUFs
On newer TUF laptops the keyboard HID device uses the same PID/VID of a USB
device that was found in ROG laptops. This patch adds the device to hid-asus
as an I2C device to enable support.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a247348c3d9..639d42708b5fc 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1640,6 +1640,9 @@ static const struct hid_device_id asus_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
> + { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
> + USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_HID_FN_LOCK },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
[ ... ]
[Severity: High]
This is a pre-existing issue, but could asus_e1239t_event() trigger a NULL
pointer dereference or use-after-free when handling a Medion E1239T device
with no input usages?
If a malicious Medion E1239T device provides a HID descriptor with no
recognized input usages, asus_input_configured() is not called, or the input
is freed by the core. This leaves drvdata->input NULL or dangling.
However, hid_hw_start() will still succeed by claiming the device as a hidraw
device. When a raw event (such as a mute key press) is received,
asus_raw_event() routes it to asus_e1239t_event().
This function unconditionally dereferences the input pointer:
drivers/hid/hid-asus.c:asus_e1239t_event() {
...
input_report_key(drvdat->input, KEY_MUTE, 1);
...
}
Is it possible for this raw event routing to dereference drvdata->input
without validating if it is non-NULL or if the input was properly claimed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=7
^ permalink raw reply
* Re: [PATCH v3 1/8] HID: asus: mitigate possible use-after-free
From: sashiko-bot @ 2026-06-13 15:47 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613153029.2559774-2-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing initialization of `fn_lock_sync_work` leading to `WARN_ON` in `cancel_work_sync()`.
--
commit 317502709e07b71f7cfc9f30cf392aa744d4f88d
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: mitigate possible use-after-free
The kbd_backlight pointer is not being set to NULL after being freed in
asus_kbd_register_leds and other portions of the driver checks that field,
therefore set the pointer to NULL after being freed.
Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..c2a5edebbd7a7 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (ret < 0) {
> /* No need to have this still around */
> devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> + drvdata->kbd_backlight = NULL;
> }
>
> return ret;
[Severity: High]
This is a pre-existing issue, but since the patch touches driver teardown
safety, does the current teardown logic in asus_remove() safely handle
uninitialized work structs?
In drivers/hid/hid-asus.c:asus_input_configured(), the work struct is
conditionally initialized:
if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
drvdata->fn_lock = true;
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
asus_kbd_set_fn_lock(hdev, true);
}
However, for some devices (like those with QUIRK_ROG_NKEY_KEYBOARD and
QUIRK_HID_FN_LOCK quirks), the HID core might skip input checks if the
device does not have inputs. If asus_input_configured() is never called,
fn_lock_sync_work remains zero-initialized.
When the device is unbound, drivers/hid/hid-asus.c:asus_remove() calls:
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
Can this trigger a WARN_ON(!work->func) in __flush_work() if
cancel_work_sync() is called on the zero-initialized work struct,
potentially causing a panic on systems with panic_on_warn=1?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=1
^ permalink raw reply
* Re: [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
From: sashiko-bot @ 2026-06-13 15:53 UTC (permalink / raw)
To: Denis Benato; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260613153029.2559774-7-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] Unvalidated `size` from an untrusted HID event directly overwrites `drvdata->wmi_size`, leading to a stack buffer overflow in `asus_wmi_work()`.
- [High] Missing `led_classdev_unregister` for `xgm_led` on `asus_probe` error paths leading to a Use-After-Free.
Pre-existing issues:
- [High] Missing `cancel_work_sync` on the `asus_probe` error path can lead to a use-after-free of the `kbd_backlight->work` workqueue structure.
--
commit 621a1df344c100c1c65a96b5fd1d410d256fafef
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
schedule_work() after the work was supposedly canceled.
Once the driver is removed and drvdata is freed by devres, the workqueue
could execute asus_sync_fn_lock() and dereference the freed drvdata.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a3437789b2341..c1a247348c3d9 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
[Severity: Critical]
Can this lead to a stack buffer overflow?
Unvalidated size from an untrusted HID event directly overwrites
drvdata->wmi_size in the commit 'HID: asus: avoid sleeping calls in atomic
context':
drivers/hid/hid-asus.c:asus_raw_event() {
...
if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
...
drvdata->wmi_size = size;
...
}
Because size comes from the transport layer, a malicious USB device can
supply an event size up to HID_MAX_BUFFER_SIZE (4096 bytes).
Later, the asynchronously scheduled asus_wmi_work() uses this oversized
wmi_size:
drivers/hid/hid-asus.c:asus_wmi_work() {
...
u8 report_data[FEATURE_KBD_REPORT_SIZE];
...
memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
...
}
Will this overflow the 64-byte stack buffer report_data?
[Severity: High]
This is a pre-existing issue, but does this error path need a call to
cancel_work_sync(&drvdata->kbd_backlight->work)?
drivers/hid/hid-asus.c:asus_probe() {
...
err_unregister_backlight:
if (drvdata->kbd_backlight) {
asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
devm_kfree(&hdev->dev, drvdata->kbd_backlight);
drvdata->kbd_backlight = NULL;
}
...
}
If a subsequent initialization step fails, the error path unregisters the
listener and frees the backlight structure.
If the work was queued prior to the failure, wouldn't the workqueue core
execute the pending work on the newly freed memory?
[Severity: High]
Does this code leave a dangling pointer in the LED subsystem list?
drivers/hid/hid-asus.c:asus_probe() {
...
err_stop_hw:
hid_hw_stop(hdev);
return ret;
}
In the commit 'HID: asus: add support for xgm led', asus_xgm_init()
allocates drvdata->xgm_led using devm_kzalloc() and registers it via
led_classdev_register().
If a subsequent initialization step fails, the error handling paths branch
to err_unregister_backlight and err_stop_hw without calling
led_classdev_unregister() for the XGM LED.
When asus_probe() returns, devres will automatically free drvdata->xgm_led,
but won't unregister the device, which could leave a dangling pointer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613153029.2559774-1-denis.benato@linux.dev?part=6
^ permalink raw reply
* Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: Antheas Kapenekakis @ 2026-06-13 15:57 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sahiko-bot
In-Reply-To: <20260613153029.2559774-5-denis.benato@linux.dev>
On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> asus_kbd_register_leds(), I noticed it registers a listener to a global list
> and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
> fails the driver returns without unregistering the listener, and the devres
> subsystem will automatically free the memory, leaving a dangling pointer
> in the global list.
>
> Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
> drivers/hid/hid-asus.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 0a6c97155549..f38b18ad65c6 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (drvdata->tp) {
> ret = asus_start_multitouch(hdev);
> if (ret)
> - goto err_stop_hw;
> + goto err_unregister_backlight;
> }
This block currently does two things, rename and init the touchpad.
Instead, you may pull the touchpad init to be below the hid_hw_start
block and keep the same bail.
> }
>
> return 0;
> +err_unregister_backlight:
> + if (drvdata->kbd_backlight) {
> + asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> + devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> + drvdata->kbd_backlight = NULL;
> + }
> err_stop_hw:
OR add asus_hid_unregister_listener(&drvdata->kbd_backlight->listener) here.
Here, you'd also drop err_unregister_backlight, devm_kfree, and
nulling. The driver is closing. It is not necessary.
err_stop_hw is only used once, you do not need a secondary tag.
> hid_hw_stop(hdev);
> return ret;
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v3 1/8] HID: asus: mitigate possible use-after-free
From: Antheas Kapenekakis @ 2026-06-13 15:59 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sashiko-bot
In-Reply-To: <20260613153029.2559774-2-denis.benato@linux.dev>
On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> The kbd_backlight pointer is not being set to NULL after being freed in
> asus_kbd_register_leds and other portions of the driver checks that field,
> therefore set the pointer to NULL after being freed.
>
> Link: https://lore.kernel.org/all/20260612130125.AAAE71F000E9@smtp.kernel.org/
>
If you do another revision, you should drop the space between the link and tags
> Suggested-by: sashiko-bot@kernel.org
> Reviewed-by: Antheas Kapenekakis <lkml@antheas.dev>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
> drivers/hid/hid-asus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d34d74df3dc0..4d7110b892bd 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -777,6 +777,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (ret < 0) {
> /* No need to have this still around */
> devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> + drvdata->kbd_backlight = NULL;
> }
>
> return ret;
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v3 6/8] HID: asus: prevent a late KEY_FN_ESC to trigger a use-after-free
From: Antheas Kapenekakis @ 2026-06-13 16:04 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sahiko-bot
In-Reply-To: <20260613153029.2559774-7-denis.benato@linux.dev>
On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> A late KEY_FN_ESC event can trigger asus_event() and unconditionally call
> schedule_work() after the work was supposedly canceled.
>
> Once the driver is removed and drvdata is freed by devres, the workqueue
> could execute asus_sync_fn_lock() and dereference the freed drvdata.
>
> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
> drivers/hid/hid-asus.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a6467172c455..a65a28137f0d 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -148,6 +148,7 @@ struct asus_drvdata {
> int battery_stat;
> bool battery_in_query;
> unsigned long battery_next_query;
> + bool fn_lock_sync_work_disabled;
> struct work_struct fn_lock_sync_work;
> bool fn_lock;
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> @@ -416,7 +417,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
> case KEY_KBDILLUMTOGGLE:
> return !asus_hid_event(ASUS_EV_BRTTOGGLE);
> case KEY_FN_ESC:
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> + if (!drvdata->fn_lock_sync_work_disabled && (drvdata->quirks & QUIRK_HID_FN_LOCK)) {
> drvdata->fn_lock = !drvdata->fn_lock;
> schedule_work(&drvdata->fn_lock_sync_work);
> }
> @@ -1497,8 +1498,10 @@ static void asus_remove(struct hid_device *hdev)
> cancel_work_sync(&drvdata->kbd_backlight->work);
> }
>
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> + if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> + drvdata->fn_lock_sync_work_disabled = true;
> cancel_work_sync(&drvdata->fn_lock_sync_work);
> + }
I am not sure this is a real bug. can _event be called during _remove?
Doesn't the core prevent that?
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> drvdata->wmi_work_disabled = true;
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: Antheas Kapenekakis @ 2026-06-13 16:15 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sahiko-bot
In-Reply-To: <20260613153029.2559774-6-denis.benato@linux.dev>
On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>
> Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
> asus_raw_event() that is called in atomic context.
>
> This commit changes when methods are called, not method parameters:
> the driver behaves as before.
Observe the new AI guidelines and add assisted by tags if necessary.
Or write the patches manually for mainline. This sentence is an AI
euphemism
Who let 1489a34e97ef through? I implemented the plumbing for WMI
callbacks with asus_hid_event last year because WMI cannot be called
through HID. Brightness events had the exact same problem.
kbd_led_work/kbd_led_update_all could be renamed and used for this. I
would rather avoid introducing a new work queue. The one I added is
enough if not too much
Antheas
> Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
> Reported-by: sahiko-bot@kernel.org
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
> drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index f38b18ad65c6..a6467172c455 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -150,6 +150,13 @@ struct asus_drvdata {
> unsigned long battery_next_query;
> struct work_struct fn_lock_sync_work;
> bool fn_lock;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + struct work_struct wmi_work;
> + bool wmi_work_disabled;
> + u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
> + int wmi_size;
> + spinlock_t wmi_lock;
> +#endif
> struct asus_xgm_led *xgm_led;
> };
>
> @@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
> return 0;
> }
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> /*
> * Send events to asus-wmi driver for handling special keys
> */
> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
> return 0;
> }
>
> +static void asus_wmi_work(struct work_struct *work)
> +{
> + struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> + u8 report_data[FEATURE_KBD_REPORT_SIZE];
> + int report_size;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
> + report_size = drvdata->wmi_size;
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> + if (ret) {
> + if (ret != -ENODEV)
> + hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> + /* Fallback: pass the raw event to the HID core */
> + hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> + report_data, report_size,
> + report_size, 1);
> + }
> +}
> +#endif
> +
> static int asus_event(struct hid_device *hdev, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> @@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
> struct hid_report *report, u8 *data, int size)
> {
> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + unsigned long flags;
> +#endif
>
> if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
> return asus_report_battery(drvdata, data, size);
> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
> * pass to userspace so it can implement its own fan control.
> */
> if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
> - int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> -
> - if (ret == 0) {
> - /* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
> + memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> + drvdata->wmi_size = size;
> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> + if (!drvdata->wmi_work_disabled) {
> + schedule_work(&drvdata->wmi_work);
> + /* Successfully handled asynchronously, block event */
> return -1;
> }
> -
> - /*
> - * Warn if asus-wmi failed (but not if it's unavailable).
> - * Let the event reach userspace in all failure cases.
> - */
> - if (ret != -ENODEV)
> - hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
> +#endif
> }
>
> /*
> @@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
> drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> + spin_lock_init(&drvdata->wmi_lock);
> +#endif
>
> if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
> ret = asus_battery_probe(hdev);
> @@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
> if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> cancel_work_sync(&drvdata->fn_lock_sync_work);
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> + drvdata->wmi_work_disabled = true;
> + cancel_work_sync(&drvdata->wmi_work);
> +#endif
> +
> if (drvdata->xgm_led)
> led_classdev_unregister(&drvdata->xgm_led->cdev);
>
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH 1/7] hid-asus: Fix up Zenbook Duo report descriptors
From: Nathan Chancellor @ 2026-06-13 16:19 UTC (permalink / raw)
To: Paolo Pisati; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-2-p.pisati@gmail.com>
Hi Paolo,
On Wed, May 13, 2026 at 06:32:42PM +0200, Paolo Pisati wrote:
> From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
>
> This is similar to the T100CHI/T90CHI keyboard dock fix.
> Without it, all reports log:
>
> Unmapped Asus vendor usagepage code 0x76
>
> Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
> ---
> drivers/hid/hid-asus.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..ce246efba74d3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -99,6 +99,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> #define QUIRK_HID_FN_LOCK BIT(14)
> +#define QUIRK_ZENBOOK_DUO_KEYBOARD BIT(15)
>
> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> QUIRK_NO_INIT_REPORTS | \
> @@ -1384,17 +1385,20 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
> rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
> }
> - /* For the T100CHI/T90CHI keyboard dock */
> - if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
> + /* For the T100CHI/T90CHI keyboard dock and Zenbook Duo 2024+ keyboards */
> + if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI | QUIRK_ZENBOOK_DUO_KEYBOARD)) {
> int rsize_orig;
> int offs;
>
> if (drvdata->quirks & QUIRK_T100CHI) {
> rsize_orig = 403;
> offs = 388;
> - } else {
> + } else if (drvdata->quirks & QUIRK_T90CHI) {
> rsize_orig = 306;
> offs = 291;
> + } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
> + rsize_orig = 257;
> + offs = 176;
> }
The way this if block is now written will cause a clang warning:
https://lore.kernel.org/202606110526.QfgiXQTQ-lkp@intel.com/
It is obviously a false positive since we know one of those branches
will be taken from the parent if statement's condition.
Consider keeping the else, maybe with a comment, to make it clear to
both humans and the compiler that all cases are covered?
} else { /* QUIRK_ZENBOOK_DUO_KEYBOARD */
rsize_orig = 257;
offs = 176;
}
> /*
> @@ -1414,7 +1418,8 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>
> hid_info(hdev, "Fixing up %s keyb report descriptor\n",
> drvdata->quirks & QUIRK_T100CHI ?
> - "T100CHI" : "T90CHI");
> + "T100CHI" : drvdata->quirks & QUIRK_T90CHI ?
> + "T90CHI" : "ZENBOOK DUO");
>
> memcpy(new_rdesc, rdesc, rsize_orig);
> *rsize = rsize_orig + 1;
> --
> 2.53.0
>
--
Cheers,
Nathan
^ permalink raw reply
* [PATCH 0/2] HID: logitech-hidpp: fix Signature M650 side button timing
From: Elliot Douglas @ 2026-06-13 17:51 UTC (permalink / raw)
To: linux-input; +Cc: lains, hadess, jikos, bentiss, linux-kernel, edouglas7358
The Logitech Signature M650 over Bluetooth exposes its side buttons in the
normal mouse report, but the reported BTN_SIDE/BTN_EXTRA events are short
click-like events emitted around button release rather than physical
press/release events with the real hold duration. The device appears to reserve
the held side-button state for a built-in gesture mode: holding a side button
long enough, or holding it while using the wheel for horizontal scrolling, can
mean the normal mouse report never emits a usable side-button press at all.
That makes the buttons unusable for standard Linux hold actions such as
push-to-talk, drag modifiers, or remapping rules that depend on key-up timing.
When HID++ 2.0 feature 0x1b04, SpecialKeysMseButtons /
REPROG_CONTROLS_V4, temporarily diverts the same controls, the device sends
diverted-control notifications with real press and release timing. This series
adds quirk-gated support for those notifications and enables it for the
Bluetooth Signature M650.
Before enabling diversion, the driver verifies that each mapped control is
present in the device's HID++ control table and is advertised as a divertable
mouse control.
The diverted M650 controls are reported as BTN_BACK and BTN_FORWARD. Logitech's
Signature M650 getting-started page labels these physical controls as
Back/Forward buttons and describes their default page-navigation behavior:
https://support.logi.com/hc/en-nz/articles/4414473810583-Getting-Started-Signature-M650
The reprogrammable-control support is per-product and parses the full HID++
divertedButtonsEvent pressed-control list, so it can support devices with more
buttons without relying on a single last-control release heuristic. Only the
Signature M650 opts in for now. Other Logitech devices should only be enabled
after their HID++ control IDs and divertedButtonsEvent behavior are captured
and verified.
There is evidence that this is not unique to the M650. A prior MX Anywhere 3
patch used the same HID++ feature to fix thumb buttons that only activated on
release, and Logitech documents side-button + wheel horizontal scrolling for
both the MX Anywhere 3/3S and Signature M650. Solaar's device reports and rules
documentation also show HID++ divertable back/forward controls on MX Master 3
and MX Master 3S class devices. This series remains conservative and only
enables the device tested here.
Tested with a Logitech Signature M650 L over Bluetooth, HID ID
0005:046D:B02A. Baseline evtest showed short release-time BTN_SIDE/BTN_EXTRA
events. Earlier local testing of the same HID++ diversion path showed real
hold-duration press/release events, including holds longer than 4 seconds for
both buttons.
Elliot Douglas (2):
HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support
HID: logitech-hidpp: enable reprogrammable buttons on Signature M650
drivers/hid/hid-logitech-hidpp.c | 236 ++++++++++++++++++++++++++++++-
1 file changed, 235 insertions(+), 1 deletion(-)
base-commit: f0866517be9345d8245d32b722574b8aecccb348
--
2.54.0
^ permalink raw reply
* [PATCH 1/2] HID: logitech-hidpp: add HID++ 2.0 reprogrammable button support
From: Elliot Douglas @ 2026-06-13 17:51 UTC (permalink / raw)
To: linux-input; +Cc: lains, hadess, jikos, bentiss, linux-kernel, edouglas7358
In-Reply-To: <20260613175109.44365-1-edouglas7358@gmail.com>
Some Logitech HID++ 2.0 mice can report diverted reprogrammable
controls through HID++ feature 0x1b04, SpecialKeysMseButtons /
REPROG_CONTROLS_V4, instead of the normal HID mouse report.
Add a quirk-gated event path for those controls. The handler temporarily
diverts verified per-product controls, parses divertedButtonsEvent as the
current pressed-control list, and reports the corresponding evdev key state
for every mapped control.
Keep the control mappings in per-product profiles so adding support for
another mouse does not change the evdev capabilities advertised by
already-supported devices.
Documentation for feature 0x1b04 describes divertedButtonsEvent as a list
of currently pressed diverted buttons, which is the event format handled
here.
Link: https://lekensteyn.nl/files/logitech/x1b04_specialkeysmsebuttons.html
Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 215 +++++++++++++++++++++++++++++++
1 file changed, 215 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 70ba1a5e40d8..24c9cfaa4f37 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -76,6 +76,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
#define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(28)
#define HIDPP_QUIRK_WIRELESS_STATUS BIT(29)
#define HIDPP_QUIRK_RESET_HI_RES_SCROLL BIT(30)
+#define HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS BIT(31)
/* These are just aliases for now */
#define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
@@ -205,6 +206,7 @@ struct hidpp_device {
struct hidpp_scroll_counter vertical_wheel_counter;
u8 wireless_feature_index;
+ u8 reprog_controls_feature_index;
int hires_wheel_multiplier;
u8 hires_wheel_feature_index;
@@ -3601,6 +3603,209 @@ static int hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
return 1;
}
+/* -------------------------------------------------------------------------- */
+/* HID++2.0 reprogrammable controls */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_REPROG_CONTROLS_V4 0x1b04
+
+#define HIDPP_REPROG_CONTROLS_GET_COUNT 0x00
+#define HIDPP_REPROG_CONTROLS_GET_CID_INFO 0x10
+#define HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING 0x30
+
+#define HIDPP_REPROG_CONTROLS_FLAG_MOUSE BIT(0)
+#define HIDPP_REPROG_CONTROLS_FLAG_DIVERT BIT(5)
+
+#define HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED BIT(0)
+#define HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT BIT(1)
+
+#define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED 0x00
+
+struct hidpp_reprog_control_mapping {
+ u16 control;
+ u16 code;
+};
+
+struct hidpp_reprog_controls_profile {
+ const struct hidpp_reprog_control_mapping *mappings;
+ unsigned int mapping_count;
+};
+
+static const struct hidpp_reprog_controls_profile *
+hidpp20_reprog_controls_get_profile(struct hidpp_device *hidpp)
+{
+ return NULL;
+}
+
+static int hidpp20_reprog_controls_get_count(struct hidpp_device *hidpp)
+{
+ struct hidpp_report response;
+ u8 feature_index = hidpp->reprog_controls_feature_index;
+ u8 cmd = HIDPP_REPROG_CONTROLS_GET_COUNT;
+ int ret;
+
+ ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd, NULL, 0,
+ &response);
+ if (ret > 0)
+ return -EPROTO;
+ if (ret)
+ return ret;
+
+ return response.fap.params[0];
+}
+
+static int hidpp20_reprog_controls_get_cid_info(struct hidpp_device *hidpp,
+ u8 index, u16 *control,
+ u8 *flags)
+{
+ struct hidpp_report response;
+ u8 feature_index = hidpp->reprog_controls_feature_index;
+ u8 cmd = HIDPP_REPROG_CONTROLS_GET_CID_INFO;
+ int ret;
+
+ ret = hidpp_send_fap_command_sync(hidpp, feature_index, cmd, &index,
+ sizeof(index), &response);
+ if (ret > 0)
+ return -EPROTO;
+ if (ret)
+ return ret;
+
+ *control = get_unaligned_be16(&response.fap.params[0]);
+ *flags = response.fap.params[4];
+
+ return 0;
+}
+
+static bool hidpp20_reprog_controls_find_control(struct hidpp_device *hidpp,
+ u16 control)
+{
+ int count, ret;
+ u16 cid;
+ u8 flags;
+ int i;
+
+ count = hidpp20_reprog_controls_get_count(hidpp);
+ if (count < 0)
+ return false;
+
+ for (i = 0; i < count; i++) {
+ ret = hidpp20_reprog_controls_get_cid_info(hidpp, i, &cid,
+ &flags);
+ if (ret)
+ return false;
+
+ if (cid == control)
+ return (flags & HIDPP_REPROG_CONTROLS_FLAG_MOUSE) &&
+ (flags & HIDPP_REPROG_CONTROLS_FLAG_DIVERT);
+ }
+
+ return false;
+}
+
+static int hidpp20_reprog_controls_set_control_reporting(struct hidpp_device *hidpp,
+ u16 control, u8 flags)
+{
+ struct hidpp_report response;
+ u8 params[5];
+
+ put_unaligned_be16(control, ¶ms[0]);
+ params[2] = flags;
+ put_unaligned_be16(control, ¶ms[3]);
+
+ return hidpp_send_fap_command_sync(hidpp,
+ hidpp->reprog_controls_feature_index,
+ HIDPP_REPROG_CONTROLS_SET_CONTROL_REPORTING,
+ params, sizeof(params), &response);
+}
+
+static void hidpp20_reprog_controls_connect(struct hidpp_device *hidpp)
+{
+ const struct hidpp_reprog_controls_profile *profile;
+ u8 flags = HIDPP_REPROG_CONTROLS_TEMPORARY_DIVERTED |
+ HIDPP_REPROG_CONTROLS_CHANGE_TEMPORARY_DIVERT;
+ unsigned int i;
+
+ if (!(hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS))
+ return;
+
+ profile = hidpp20_reprog_controls_get_profile(hidpp);
+ if (!profile)
+ return;
+
+ if (hidpp_root_get_feature(hidpp, HIDPP_PAGE_REPROG_CONTROLS_V4,
+ &hidpp->reprog_controls_feature_index))
+ return;
+
+ for (i = 0; i < profile->mapping_count; i++) {
+ u16 control = profile->mappings[i].control;
+
+ if (!hidpp20_reprog_controls_find_control(hidpp, control))
+ continue;
+
+ hidpp20_reprog_controls_set_control_reporting(hidpp, control, flags);
+ }
+}
+
+static int hidpp20_reprog_controls_raw_event(struct hidpp_device *hidpp,
+ u8 *data, int size)
+{
+ const struct hidpp_reprog_controls_profile *profile;
+ const struct hidpp_reprog_control_mapping *mapping;
+ struct hidpp_report *report = (struct hidpp_report *)data;
+ u16 controls[4];
+ bool pressed;
+ unsigned int i, j;
+
+ if (!(hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS) ||
+ !hidpp->input ||
+ hidpp->reprog_controls_feature_index == 0xff)
+ return 0;
+
+ profile = hidpp20_reprog_controls_get_profile(hidpp);
+ if (!profile)
+ return 0;
+
+ if (size < HIDPP_REPORT_LONG_LENGTH ||
+ report->fap.feature_index != hidpp->reprog_controls_feature_index ||
+ report->fap.funcindex_clientid != HIDPP_REPROG_CONTROLS_EVENT_DIVERTED)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(controls); i++)
+ controls[i] = get_unaligned_be16(&report->fap.params[i * 2]);
+
+ for (i = 0; i < profile->mapping_count; i++) {
+ mapping = &profile->mappings[i];
+ pressed = false;
+
+ for (j = 0; j < ARRAY_SIZE(controls); j++) {
+ if (controls[j] == mapping->control) {
+ pressed = true;
+ break;
+ }
+ }
+
+ input_report_key(hidpp->input, mapping->code, pressed);
+ }
+
+ input_sync(hidpp->input);
+
+ return 1;
+}
+
+static void hidpp20_reprog_controls_populate_input(struct hidpp_device *hidpp,
+ struct input_dev *input_dev)
+{
+ const struct hidpp_reprog_controls_profile *profile;
+ unsigned int i;
+
+ profile = hidpp20_reprog_controls_get_profile(hidpp);
+ if (!profile)
+ return;
+
+ for (i = 0; i < profile->mapping_count; i++)
+ input_set_capability(input_dev, EV_KEY, profile->mappings[i].code);
+}
+
static void hidpp10_extra_mouse_buttons_populate_input(
struct hidpp_device *hidpp, struct input_dev *input_dev)
{
@@ -3859,6 +4064,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS)
hidpp10_extra_mouse_buttons_populate_input(hidpp, input);
+
+ if (hidpp->quirks & HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS)
+ hidpp20_reprog_controls_populate_input(hidpp, input);
}
static int hidpp_input_configured(struct hid_device *hdev,
@@ -3971,6 +4179,10 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
return ret;
}
+ ret = hidpp20_reprog_controls_raw_event(hidpp, data, size);
+ if (ret != 0)
+ return ret;
+
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
ret = hidpp10_consumer_keys_raw_event(hidpp, data, size);
if (ret != 0)
@@ -4264,6 +4476,8 @@ static void hidpp_connect_event(struct work_struct *work)
return;
}
+ hidpp20_reprog_controls_connect(hidpp);
+
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
ret = hidpp10_consumer_keys_connect(hidpp);
if (ret)
@@ -4436,6 +4650,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp->hid_dev = hdev;
hidpp->name = hdev->name;
hidpp->quirks = id->driver_data;
+ hidpp->reprog_controls_feature_index = 0xff;
hid_set_drvdata(hdev, hidpp);
ret = hid_parse(hdev);
--
2.54.0
^ permalink raw reply related
* [PATCH 2/2] HID: logitech-hidpp: enable reprogrammable buttons on Signature M650
From: Elliot Douglas @ 2026-06-13 17:51 UTC (permalink / raw)
To: linux-input; +Cc: lains, hadess, jikos, bentiss, linux-kernel, edouglas7358
In-Reply-To: <20260613175109.44365-1-edouglas7358@gmail.com>
The Bluetooth Signature M650 exposes its side buttons through the normal
mouse report, but the observed events are short click-like events emitted
around release rather than physical press/release state.
The device appears to use the held side-button state for its built-in
gesture and side-button + wheel horizontal-scroll mode. As a result,
holding a side button long enough can prevent the normal mouse report from
emitting a usable button event at all.
HID++ REPROG_CONTROLS_V4 diversion for control IDs 0x0053 and 0x0056
provides real press and release timing for those same controls. Logitech
documents the Signature M650 side buttons as Back/Forward buttons, so
report the diverted controls as BTN_BACK and BTN_FORWARD.
Link: https://support.logi.com/hc/en-nz/articles/4414473810583-Getting-Started-Signature-M650
Signed-off-by: Elliot Douglas <edouglas7358@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 24c9cfaa4f37..80108778ee80 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3621,6 +3621,9 @@ static int hidpp10_extra_mouse_buttons_raw_event(struct hidpp_device *hidpp,
#define HIDPP_REPROG_CONTROLS_EVENT_DIVERTED 0x00
+#define HIDPP_REPROG_CONTROL_M650_BACK 0x0053
+#define HIDPP_REPROG_CONTROL_M650_FORWARD 0x0056
+
struct hidpp_reprog_control_mapping {
u16 control;
u16 code;
@@ -3631,9 +3634,24 @@ struct hidpp_reprog_controls_profile {
unsigned int mapping_count;
};
+static const struct hidpp_reprog_control_mapping m650_reprog_control_mappings[] = {
+ { HIDPP_REPROG_CONTROL_M650_BACK, BTN_BACK },
+ { HIDPP_REPROG_CONTROL_M650_FORWARD, BTN_FORWARD },
+};
+
+static const struct hidpp_reprog_controls_profile m650_reprog_controls_profile = {
+ .mappings = m650_reprog_control_mappings,
+ .mapping_count = ARRAY_SIZE(m650_reprog_control_mappings),
+};
+
static const struct hidpp_reprog_controls_profile *
hidpp20_reprog_controls_get_profile(struct hidpp_device *hidpp)
{
+ switch (hidpp->hid_dev->product) {
+ case 0xb02a:
+ return &m650_reprog_controls_profile;
+ }
+
return NULL;
}
@@ -4921,7 +4939,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* MX Vertical mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb020) },
{ /* Signature M650 over Bluetooth */
- HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb02a) },
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb02a),
+ .driver_data = HIDPP_QUIRK_HIDPP_REPROG_CONTROLS_BTNS },
{ /* MX Master 3 mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
{ /* MX Anywhere 3 mouse over Bluetooth */
--
2.54.0
^ permalink raw reply related
* [PATCH] Input: edt-ft5x06: correct CONFIG_DEBUG_FS macro name in comment
From: Ethan Nelson-Moore @ 2026-06-13 20:50 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Ethan Nelson-Moore
A comment in the edt-ft5x06 driver incorrectly refers to CONFIG_DEBUGFS
instead of CONFIG_DEBUG_FS. Correct it.
Discovered while searching for CONFIG_* symbols referenced in code but
not defined in any Kconfig file.
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d3b1177185a3..410be061965e 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -825,7 +825,7 @@ static void edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
{
}
-#endif /* CONFIG_DEBUGFS */
+#endif /* CONFIG_DEBUG_FS */
static int edt_ft5x06_ts_identify(struct i2c_client *client,
struct edt_ft5x06_ts_data *tsdata)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] Input: edt-ft5x06: correct CONFIG_DEBUG_FS macro name in comment
From: sashiko-bot @ 2026-06-13 20:59 UTC (permalink / raw)
To: Ethan Nelson-Moore; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260613205042.80181-1-enelsonmoore@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Imbalanced disable_irq() causes the touchscreen to remain disabled after resume if suspended while in factory mode.
--
commit c833a423ddc21c60c7ceaf7761d2c4e876b25fee
Author: Ethan Nelson-Moore <enelsonmoore@gmail.com>
Input: edt-ft5x06: correct CONFIG_DEBUG_FS macro name in comment
A comment in the edt-ft5x06 driver incorrectly refers to CONFIG_DEBUGFS
instead of CONFIG_DEBUG_FS. Correct it.
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index d3b1177185a3b..410be061965ec 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -825,7 +825,7 @@ static void edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
> {
> }
>
> -#endif /* CONFIG_DEBUGFS */
> +#endif /* CONFIG_DEBUG_FS */
[Severity: High]
This is a pre-existing issue, but I noticed an IRQ depth imbalance that
causes the touchscreen to remain disabled after resume if suspended while in
factory mode.
If the device is placed into factory mode via debugfs,
edt_ft5x06_factory_mode() calls disable_irq(), increasing the IRQ disable
depth to 1. During system suspend, edt_ft5x06_ts_suspend() unconditionally
calls disable_irq(), increasing the depth to 2.
On resume, edt_ft5x06_ts_resume() calls enable_irq() (reducing depth to 1),
but then it invokes edt_ft5x06_factory_mode() to restore state:
drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_ts_resume() {
...
if (tsdata->factory_mode)
ret = edt_ft5x06_factory_mode(tsdata);
...
}
Because edt_ft5x06_factory_mode() unconditionally calls disable_irq()
again, the depth goes back to 2:
drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_factory_mode() {
...
disable_irq(client->irq);
...
}
When the user later switches back to work mode, edt_ft5x06_work_mode() only
calls enable_irq() once. Does this leave the IRQ permanently disabled,
making the touchscreen entirely unusable?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613205042.80181-1-enelsonmoore@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: apple_z2 - bound the device-reported packet length
From: Bryam Vargas via B4 Relay @ 2026-06-13 21:42 UTC (permalink / raw)
To: Dmitry Torokhov, Sasha Finkelstein
Cc: linux-arm-kernel, Neal Gompa, linux-input, asahi, linux-kernel,
Sven Peter, Janne Grunau
From: Bryam Vargas <hexlabsecurity@proton.me>
apple_z2_read_packet() takes a 16-bit length from the touch controller's
interrupt-data reply (rx_buf[1..2]) and, only rounded down to a multiple of
four, uses it as the size of the second SPI transfer into the fixed-size
rx_buf:
pkt_len = (get_unaligned_le16(z2->rx_buf + 1) + 8) & 0xfffffffc;
error = spi_read(z2->spidev, z2->rx_buf, pkt_len);
rx_buf is a fixed 4000-byte buffer, but pkt_len is fully controlled by the
device and is never checked against it, so a malicious, malfunctioning or
counterfeit controller (or an interposer on the SPI bus) that reports a
large length makes spi_read() write up to 65540 device-supplied bytes into
the 4000-byte buffer -- a controller-driven heap out-of-bounds write of up
to about 61 KiB. The recently added reply-type check only validates
rx_buf[0], not the length.
Reject any packet whose length exceeds the receive buffer.
Fixes: 471a92f8a21a ("Input: apple_z2 - add a driver for Apple Z2 touchscreens")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reachable on every touch interrupt once the controller is booted
(apple_z2_irq -> apple_z2_read_packet).
Verified with a faithful in-kernel KASAN litmus (the verbatim 4000-byte
allocation, the exact pkt_len computation, and the spi_read transfer modelled
as a memset of pkt_len controller bytes), CONFIG_KASAN_INLINE=y:
Arm A, reported length 0x1000 -> pkt_len 4104:
BUG: KASAN: slab-out-of-bounds in apple_z2_read_packet
Write of size 4104 ... located 0 bytes inside of allocated 4000-byte region
... which belongs to the cache kmalloc-4k of size 4096
Arm B, with this patch (length rejected): clean
Arm C, benign length: clean
AddressSanitizer (x86_64 and i386), reported length 0xffff -> pkt_len 65540:
heap-buffer-overflow WRITE of size 65540, both ABIs.
Reproducer and full logs available on request.
---
drivers/input/touchscreen/apple_z2.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
index 271ababf0ad5..ff9ff97be185 100644
--- a/drivers/input/touchscreen/apple_z2.c
+++ b/drivers/input/touchscreen/apple_z2.c
@@ -22,6 +22,7 @@
#define APPLE_Z2_TOUCH_MOVED 4
#define APPLE_Z2_CMD_READ_INTERRUPT_DATA 0xEB
#define APPLE_Z2_REPLY_INTERRUPT_DATA 0xE1
+#define APPLE_Z2_MAX_PACKET 4000
#define APPLE_Z2_HBPP_CMD_BLOB 0x3001
#define APPLE_Z2_FW_MAGIC 0x5746325A
#define LOAD_COMMAND_INIT_PAYLOAD 0
@@ -147,6 +148,8 @@ static int apple_z2_read_packet(struct apple_z2 *z2)
return 0;
pkt_len = (get_unaligned_le16(z2->rx_buf + 1) + 8) & 0xfffffffc;
+ if (pkt_len > APPLE_Z2_MAX_PACKET)
+ return -EMSGSIZE;
error = spi_read(z2->spidev, z2->rx_buf, pkt_len);
if (error)
@@ -363,7 +366,7 @@ static int apple_z2_probe(struct spi_device *spi)
if (!z2->tx_buf)
return -ENOMEM;
/* 4096 will end up being rounded up to 8192 due to devres header */
- z2->rx_buf = devm_kzalloc(dev, 4000, GFP_KERNEL);
+ z2->rx_buf = devm_kzalloc(dev, APPLE_Z2_MAX_PACKET, GFP_KERNEL);
if (!z2->rx_buf)
return -ENOMEM;
---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260613-b4-disp-b1926f1a-caad8f942af9
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related
* Re: [PATCH] Input: apple_z2 - bound the device-reported packet length
From: sashiko-bot @ 2026-06-13 21:53 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260613-b4-disp-b1926f1a-v1-1-3a277b7c0cfa@proton.me>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check on the number of fingers (`nfingers`) reported by the device in `apple_z2_parse_touches` leads to a kernel heap out-of-bounds read and potential information leak.
- [High] Concurrent execution of `apple_z2_read_packet()` from the probe/resume thread and the IRQ handler causes a data race and shared SPI buffer corruption.
- [Medium] If `apple_z2_boot()` times out waiting for the boot IRQ, it returns directly without disabling the IRQ or reverting the hardware reset state, leading to state inconsistencies.
- [Low] The input device is registered and exposed to userspace before the hardware is initialized and firmware is uploaded, violating input subsystem readiness rules.
--
commit 0ef574cc6b6df1efb30f7e523d3cf505e551ae3c
Author: Bryam Vargas <hexlabsecurity@proton.me>
Input: apple_z2 - bound the device-reported packet length
apple_z2_read_packet() takes a 16-bit length from the touch controller's
interrupt-data reply and uses it as the size of the second SPI transfer into
the fixed-size rx_buf. This bounds the device-reported packet length to reject
any packet whose length exceeds the receive buffer.
> diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
> index 271ababf0ad55..ff9ff97be185a 100644
> --- a/drivers/input/touchscreen/apple_z2.c
> +++ b/drivers/input/touchscreen/apple_z2.c
[ ... ]
> @@ -147,6 +148,8 @@ static int apple_z2_read_packet(struct apple_z2 *z2)
> return 0;
>
> pkt_len = (get_unaligned_le16(z2->rx_buf + 1) + 8) & 0xfffffffc;
> + if (pkt_len > APPLE_Z2_MAX_PACKET)
> + return -EMSGSIZE;
>
> error = spi_read(z2->spidev, z2->rx_buf, pkt_len);
> if (error)
[Severity: High]
This isn't a bug introduced by this patch, but does apple_z2_parse_touches()
lack a bounds check on the number of fingers?
It appears that apple_z2_parse_touches() reads the unvalidated nfingers count
from msg[APPLE_Z2_NUM_FINGERS_OFFSET] and blindly iterates over it:
drivers/input/touchscreen/apple_z2.c:apple_z2_parse_touches() {
...
nfingers = msg[APPLE_Z2_NUM_FINGERS_OFFSET];
fingers = (struct apple_z2_finger *)(msg + APPLE_Z2_FINGERS_OFFSET);
for (i = 0; i < nfingers; i++) {
...
}
Could a spoofed device set nfingers to 255, causing the loop to read
out-of-bounds past the rx_buf allocation, and potentially leading to an
information leak when those bytes are parsed as touch coordinates?
[Severity: High]
This is a pre-existing issue, but is there a data race when
apple_z2_read_packet() is executed concurrently?
In apple_z2_boot(), the device interrupt is unmasked via enable_irq() before
firmware is uploaded. Then in apple_z2_upload_firmware(), the driver sets
z2->booted = true and manually calls apple_z2_read_packet().
If the device asserts the interrupt concurrently, the threaded IRQ handler
apple_z2_irq() will wake up, observe z2->booted == true, and also invoke
apple_z2_read_packet().
Since there is no lock protecting apple_z2_read_packet(), could this
concurrently overwrite z2->tx_buf, mutate z2->index_parity, and corrupt the
shared SPI buffers?
[ ... ]
> @@ -363,7 +366,7 @@ static int apple_z2_probe(struct spi_device *spi)
> if (!z2->tx_buf)
> return -ENOMEM;
> /* 4096 will end up being rounded up to 8192 due to devres header */
> - z2->rx_buf = devm_kzalloc(dev, 4000, GFP_KERNEL);
> + z2->rx_buf = devm_kzalloc(dev, APPLE_Z2_MAX_PACKET, GFP_KERNEL);
> if (!z2->rx_buf)
> return -ENOMEM;
[Severity: Medium]
This is a pre-existing issue, but does apple_z2_boot() miss cleanup when
wait_for_completion_timeout() fails?
In apple_z2_boot(), the IRQ enable depth is incremented and the device is
taken out of reset. If it times out waiting for the boot_irq completion, it
directly returns -ETIMEDOUT:
drivers/input/touchscreen/apple_z2.c:apple_z2_boot() {
...
if (!wait_for_completion_timeout(&z2->boot_irq, msecs_to_jiffies(20)))
return -ETIMEDOUT;
...
}
If this happens during apple_z2_resume(), devm teardown does not occur. Does
this leave the IRQ permanently enabled and the reset GPIO active, leading to
state inconsistency?
[Severity: Low]
This is a pre-existing issue, but is the input device registered before the
hardware is fully initialized?
In apple_z2_probe(), input_register_device() is called before apple_z2_boot()
has completed:
drivers/input/touchscreen/apple_z2.c:apple_z2_probe() {
...
error = input_register_device(z2->input_dev);
if (error)
return dev_err_probe(dev, error, "unable to register input device\n");
/* Wait for device reset to finish */
usleep_range(5000, 10000);
error = apple_z2_boot(z2);
...
}
If apple_z2_boot() fails, the momentarily registered device is removed. Does
this expose an uninitialized device to userspace prematurely?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-b1926f1a-v1-1-3a277b7c0cfa@proton.me?part=1
^ permalink raw reply
* Re: [PATCH] Input: apple_z2 - bound the device-reported packet length
From: Sasha Finkelstein @ 2026-06-13 21:57 UTC (permalink / raw)
To: hexlabsecurity
Cc: Dmitry Torokhov, linux-arm-kernel, Neal Gompa, linux-input, asahi,
linux-kernel, Sven Peter, Janne Grunau
In-Reply-To: <20260613-b4-disp-b1926f1a-v1-1-3a277b7c0cfa@proton.me>
> On Jun 13, 2026, at 23:42, Bryam Vargas via B4 Relay <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> [...]
> Reject any packet whose length exceeds the receive buffer.
>
Reviewed-by: Sasha Finkelstein <k@chaosmail.tech>
Although, if you reject the packet, the datastream will be desynced and
the device should be reset before it can operate again.
^ permalink raw reply
* [PATCH] Input: synaptics-rmi4 - correct CONFIG_RMI4_F34 macro name in comment
From: Ethan Nelson-Moore @ 2026-06-13 22:21 UTC (permalink / raw)
To: Dmitry Torokhov, Marge Yang, André Apitzsch,
Ethan Nelson-Moore, linux-input
A comment in the synaptics-rmi4 driver incorrectly refers to
CONFIG_RMI4_F34 instead of CONFIG_RMI4_F34. Correct it.
Discovered while searching for CONFIG_* symbols referenced in code but
not defined in any Kconfig file.
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
drivers/input/rmi4/rmi_driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index e84495caab15..7d0ae80dc8ea 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -127,7 +127,7 @@ static inline int rmi_f34_create_sysfs(struct rmi_device *rmi_dev)
static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev)
{
}
-#endif /* CONFIG_RMI_F34 */
+#endif /* CONFIG_RMI4_F34 */
extern struct rmi_function_handler rmi_f01_handler;
extern struct rmi_function_handler rmi_f03_handler;
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox