* Re: hid-sensor-custom: async notification for input-report sysfs attributes? (human presence / attention sensors)
From: srinivas pandruvada @ 2026-07-24 15:30 UTC (permalink / raw)
To: Daniel Paul Perinchery, Jiri Kosina, Jonathan Cameron,
Benjamin Tissoires
Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <CA+8Hp=M3ynUHzcDshLup5G+HDs0XoWkav0R5k-00m+O7LbOv1w@mail.gmail.com>
Hi,
On Fri, 2026-07-24 at 12:42 +0530, Daniel Paul Perinchery wrote:
> Hi,
>
> I've been investigating why userspace can't efficiently watch a
> hid-sensor-custom input-report value (specifically a human-presence
> sensor, HID usage 2000e1, on an AMD Sensor Fusion Hub / amd_sfh
> platform) for changes without polling, and wanted to check whether
> there's interest in closing this gap before I try to write anything.
>
> Setup: Lenovo IdeaPad Pro 5 14AKP10, AMD Ryzen AI 7 350, kernel
> 7.0.13
> (Debian/Parrot, but this is stock hid-sensor-custom.c). Presence and
> gaze/attention are exposed as separate logical sensors under the same
> physical hub (HID-SENSOR-2000e1.x.auto), each with the usual
> auto-generated input-N-<usage>-value sysfs attribute.
>
> What I found tracing show_value() in hid-sensor-custom.c: for input
> reports, it calls sensor_hub_input_attr_get_raw_value(..., report_id,
> SENSOR_HUB_SYNC, false) -- i.e. every read of the -value file issues
> a
> synchronous GET_REPORT to the hub and returns the live answer.
> There's
> no cached field backing that attribute, so there's nothing for a
> notify-on-change mechanism to attach to on that path.
>
> Separately, hid_sensor_capture_sample() *is* a genuine async callback
> fired on hub-pushed input reports, but as far as I can tell it only
> feeds the raw kfifo behind the driver's misc-device streaming
> interface (custom_dev), gated on test_bit(0,
> &sensor_inst->misc_opened). It doesn't appear to update any per-field
> cached value, so it isn't reachable from the sysfs show_value() path
> either.
>
> Net effect: there's no way for a userspace consumer to poll()/
> select() on the -value file and be woken only on real hardware
> events.
I have a test program for poll(). Attached poll part.
#define DEVICE_PATH "/dev/HID-SENSOR-2000e1.2.auto"
#define BUFFER_SIZE 256
int main(int argc, char *argv[])
{
int fd;
struct pollfd pfd;
char buffer[BUFFER_SIZE];
int ret;
struct sigaction sa;
if (argc < 2) {
printf("syntax %s sample_interval\n", argv[0]);
exit(0);
}
// 1. Set up Ctrl+C signal handler
sa.sa_handler = handle_sigint;
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0; // Don't use SA_RESTART so poll() unblocks on
signal
if (sigaction(SIGINT, &sa, NULL) < 0) {
perror("Error setting up signal handler");
return -1;
}
enable_sensor(1);
// 2. Open the misc device
fd = open(DEVICE_PATH, O_RDONLY | O_NONBLOCK);
if (fd < 0) {
perror("Failed to open device");
return -1;
}
// 3. Configure poll structure
pfd.fd = fd;
pfd.events = POLLIN;
printf("Polling loop started. Press Ctrl+C to exit
safely.\n");
// 4. Main event loop
while (keep_running) {
printf("Waiting for data...\n");
ret = poll(&pfd, 1, -1);
if (ret < 0) {
// Signal interrupted poll, loop will exit if
it was SIGINT
continue;
}
if (ret == 0) {
// Timeout reached with no data
continue;
}
// 5. Handle readable data event
if (pfd.revents & POLLIN) {
rx_tsc = read_rdtsc();
ssize_t bytes_read = read(fd, buffer,
sizeof(buffer) - 1);
if (bytes_read > 0) {
time_t tm;
buffer[bytes_read] = '\0'; // Null-
terminate string
printf("tm:%ld Received %ld bytes:
%s\n", time(&tm), (long)bytes_read, buffer);
read_time_stamp(buffer, bytes_read);
} else if (bytes_read < 0) {
perror("Read error");
}
}
}
// 6. Cleanup
printf("\nExiting program cleanly...\n");
close(fd);
enable_sensor(0);
return 0;
}
Does it help.
Thanks,
Srinivas
> The hub itself is already configured for threshold-based reporting
> (property-reporting-state = 2, i.e. "Threshold Events") in my case,
> so
> the inefficiency is entirely on the kernel/sysfs side -- userspace
> has
> to fall back to timed re-reads of a synchronous, firmware-round-trip
> attribute, which is more expensive per-read than a typical cached
> sysfs value and awkward to size a safe polling interval for.
>
> This seems like a real gap for the presence/proximity use case
> specifically (walk-away lock, auto-dim, etc.), which is inherently
> event-oriented and is exactly the kind of feature vendors implement
> natively on Windows via push notifications from the same class of
> sensor.
>
> Before I put together a patch I wanted to ask: is there interest in
> adding async caching + sysfs_notify() (or sysfs_notify_dirent(), if
> attrs end up carrying a stored kernfs_node) for input-report values
> specifically, sourced from hid_sensor_capture_sample()'s existing
> async path? Rough shape as I see it:
>
> - add a cached "last input value" (+ a bool "valid") per relevant
> field in struct hid_sensor_custom_field
> - in hid_sensor_capture_sample(), in addition to (or instead of,
> behind a config/attr) feeding the misc-device fifo, decode and store
> the value for the matching field, then sysfs_notify() its -value
> attribute
> - leave show_value()'s SENSOR_HUB_SYNC path untouched for feature
> reports and for any input attrs that never get an async report, so
> nothing regresses for existing consumers
>
> Happy to be told this already exists via a different interface I've
> missed (I did look at whether this should just go through the IIO
> buffered/trigger interface instead, given some other hid-sensor-*
> drivers are IIO-backed -- but hid-sensor-custom exposes vendor/custom
> usages directly via this sysfs shape rather than IIO channels, so I
> wasn't sure that's the right layer to add it at). Also open to being
> told the misc-device/fifo interface is meant to be the async
> consumption path already and userspace tools should be using that
> instead of polling -value -- if so, is there existing tooling/docs
> for
> that path I should be pointing presence-detection use cases at?
>
> Not attaching a patch yet since I'd rather get a read on the right
> approach (and whether it's wanted at all) before writing one.
>
> Thanks,
> Daniel Paul
^ permalink raw reply
* Re: [PATCH v2] HID: lg4ff: validate report length before fixed offsets
From: sashiko-bot @ 2026-07-24 15:28 UTC (permalink / raw)
To: Jiancheng Huang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260724151317.1419574-1-jchuang@seu.edu.cn>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Force feedback is initialized on the input device after it has already been registered and made visible to userspace, violating input subsystem initialization order.
- [High] Use-after-free of the lg4ff device properties (`entry`) during device removal or probe failure, as the properties are freed before hardware I/O and input devices are fully unregistered.
- [Low] Data race on `entry->wdata.combine` between sysfs store and interrupt context raw event handler.
--
--- Patch [1]: [PATCH v2] HID: lg4ff: validate report length before fixed offsets ---
commit 64ee3d21ef774099318a4223423dc4d5d4eac310
Author: Jiancheng Huang <jchuang@seu.edu.cn>
HID: lg4ff: validate report length before fixed offsets
lg4ff_raw_event() rewrites fixed report offsets when combined pedals are
enabled. It currently assumes that each product report contains every
source and destination byte used by the rewrite.
Return without rewriting a short report before each product-specific
access. Apply the same bound to the computed offset path.
[Severity: High]
This isn't a bug introduced by this patch, but force feedback is initialized on
the input device after it has already been registered.
Looking at drivers/hid/hid-lg.c, lg_probe() registers the input device before
force feedback capabilities are set up:
drivers/hid/hid-lg.c:lg_probe() {
...
ret = hid_hw_start(hdev, connect_mask);
...
if (drv_data->quirks & LG_FF4)
ret = lg4ff_init(hdev);
...
}
Because hid_hw_start() exposes the device to userspace, could userspace (e.g.
udev, games) open the device and observe an incomplete capability set before
lg4ff_init() modifies dev->ffbit and installs the FF callbacks?
[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk for the lg4ff
device properties during device removal?
In drivers/hid/hid-lg.c, the device properties are freed before hardware I/O
and input devices are fully unregistered:
drivers/hid/hid-lg.c:lg_remove() {
struct lg_drv_data *drv_data = hid_get_drvdata(hdev);
if (drv_data->quirks & LG_FF4)
lg4ff_deinit(hdev);
hid_hw_stop(hdev);
kfree(drv_data);
}
Since lg4ff_deinit() sets drv_data->device_props = NULL and frees the entry,
if an interrupt fires before hid_hw_stop() executes, could lg4ff_raw_event()
cache the entry pointer just before it is freed, leading to a use-after-free?
Additionally, could lg4ff_play() be concurrently invoked via userspace ioctl
while the input device is still registered, resulting in a similar
use-after-free?
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index e901fdb7d033c..62f99a93210c9 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -336,6 +336,8 @@ int lg4ff_raw_event(struct hid_device *hdev, struct hid_report *report,
> if (entry->wdata.combine) {
[Severity: Low]
This is a pre-existing issue, but does reading entry->wdata.combine from
interrupt context without locks or READ_ONCE() introduce a data race?
The sysfs store function (lg4ff_combine_store) writes to this shared boolean
state without locks or WRITE_ONCE(). Could this lead to a transient read of an
inconsistent state if a user writes to the sysfs node while the device is
actively sending events?
> switch (entry->wdata.product_id) {
> case USB_DEVICE_ID_LOGITECH_WHEEL:
> + if (size < 7)
> + return 0;
> rd[5] = rd[3];
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724151317.1419574-1-jchuang@seu.edu.cn?part=1
^ permalink raw reply
* [PATCH v2] HID: lg4ff: validate report length before fixed offsets
From: Jiancheng Huang @ 2026-07-24 15:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
lg4ff_raw_event() rewrites fixed report offsets when combined pedals are
enabled. It currently assumes that each product report contains every
source and destination byte used by the rewrite.
Return without rewriting a short report before each product-specific
access. Apply the same bound to the computed offset path.
Fixes: c832f86effbc ("HID: hid-logitech: Add combined pedal support Logitech wheels")
Signed-off-by: Jiancheng Huang <jchuang@seu.edu.cn>
Assisted-by: Codex:gpt-5.6-luna
---
Changes in v2:
- Add the relevant public mailing lists to Cc; no source changes.
Evidence (v7.2-rc4 KUnit/KASAN oracle under bounded QEMU):
Source: confirmed/hid_lg4ff_short_report_oob_raw_excerpt.log
[ 2.638792] KTAP version 1
[ 2.638888] 1..1
[ 2.641100] KTAP version 1
[ 2.641235] # Subtest: hid-lg4ff-exploration
[ 2.641784] # module: hid_logitech
[ 2.641962] 1..1
[ 2.644581] ==================================================================
[ 2.645008] BUG: KASAN: slab-out-of-bounds in lg4ff_raw_event+0x2fd/0x410
[ 2.645369] Read of size 1 at addr ffff88800229fc63 by task kunit_try_catch/67
[ 2.645369]
[ 2.645369] CPU: 1 UID: 0 PID: 67 Comm: kunit_try_catch Tainted: G N 7.2.0-rc4-dirty #3 PREEMPT(full)
[ 2.645369] Tainted: [N]=TEST
[ 2.645369] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 2.645369] Call Trace:
[ 2.645369] <TASK>
[ 2.645369] dump_stack_lvl+0x53/0x70
[ 2.645369] print_report+0xd0/0x630
[ 2.645369] ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[ 2.645369] ? lg4ff_raw_event+0x2fd/0x410
[ 2.645369] kasan_report+0xe5/0x120
[ 2.645369] ? lg4ff_raw_event+0x2fd/0x410
[ 2.645369] lg4ff_raw_event+0x2fd/0x410
[ 2.645369] lg4ff_short_report_oob_test+0x14e/0x250
drivers/hid/hid-lg4ff.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index e901fdb7d..62f99a932 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -336,6 +336,8 @@ int lg4ff_raw_event(struct hid_device *hdev, struct hid_report *report,
if (entry->wdata.combine) {
switch (entry->wdata.product_id) {
case USB_DEVICE_ID_LOGITECH_WHEEL:
+ if (size < 7)
+ return 0;
rd[5] = rd[3];
rd[6] = 0x7F;
return 1;
@@ -343,10 +345,14 @@ int lg4ff_raw_event(struct hid_device *hdev, struct hid_report *report,
case USB_DEVICE_ID_LOGITECH_WINGMAN_FFG:
case USB_DEVICE_ID_LOGITECH_MOMO_WHEEL:
case USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2:
+ if (size < 6)
+ return 0;
rd[4] = rd[3];
rd[5] = 0x7F;
return 1;
case USB_DEVICE_ID_LOGITECH_DFP_WHEEL:
+ if (size < 7)
+ return 0;
rd[5] = rd[4];
rd[6] = 0x7F;
return 1;
@@ -366,6 +372,8 @@ int lg4ff_raw_event(struct hid_device *hdev, struct hid_report *report,
}
/* Compute a combined axis when wheel does not supply it */
+ if (size <= offset + 1)
+ return 0;
rd[offset] = (0xFF + rd[offset] - rd[offset+1]) >> 1;
rd[offset+1] = 0x7F;
return 1;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
From: sashiko-bot @ 2026-07-24 14:39 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260724142703.55813-1-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Error paths in `sony_probe()` fail to remove the device context from `sony_device_list`, leading to a global use-after-free.
- [High] System suspend breaks the periodic keep-alive timer, permanently disconnecting the GHL guitar dongle.
- [Low] URB DMA buffers are allocated using `devm_kzalloc()`, violating DMA API cache-line alignment requirements.
--
--- Patch [1]: [PATCH v2] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind ---
commit 0fb03a05b4d9cc719ee1096b892767332261493d
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
For GHL (Guitar Hero Live) dongles, sony_probe() arms a periodic timer
that submits a URB, which on completion re-arms the timer.
sony_remove() drained the timer with timer_delete_sync() and then freed
the URB, which did not block re-arming or kill in-flight URBs. This allowed
the timer to re-arm on freed memory, causing a use-after-free.
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index e75246d29e16d..77ab30303080e 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -2424,7 +2424,9 @@ static void sony_remove(struct hid_device *hdev)
> struct sony_sc *sc = hid_get_drvdata(hdev);
>
> if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
> - timer_delete_sync(&sc->ghl_poke_timer);
> + /* poison, not kill: a pending timer must not re-submit during teardown */
> + usb_poison_urb(sc->ghl_urb);
> + timer_shutdown_sync(&sc->ghl_poke_timer);
> usb_free_urb(sc->ghl_urb);
> }
[Severity: High]
This isn't a bug introduced by this patch, but error paths in sony_probe()
fail to remove the device context from sony_device_list, leading to a global
use-after-free.
When hid_hw_start() triggers sony_input_configured(), the device context sc
is added to the global sony_device_list. If a subsequent operation in
sony_probe() fails (for example, usb_alloc_urb() returns NULL), the function
jumps to the err label:
drivers/hid/hid-sony.c:sony_probe() {
...
err:
usb_free_urb(sc->ghl_urb);
hid_hw_stop(hdev);
return ret;
}
Can this lead to a crash when the devm subsystem frees sc upon returning
the error? If the next Sony HID device is plugged in, it seems
sony_check_add() will iterate over sony_device_list and dereference the
freed sc pointer. Should this error path call sony_remove_dev_list(sc)
and sony_release_device_id(sc)?
[Severity: High]
This isn't a bug introduced by this patch, but system suspend breaks the
periodic keep-alive timer, permanently disconnecting the GHL guitar dongle.
During system suspend, usb_submit_urb() fails synchronously. When this
happens, the timer callback exits without re-arming the timer:
drivers/hid/hid-sony.c:ghl_magic_poke() {
...
ret = usb_submit_urb(sc->ghl_urb, GFP_ATOMIC);
if (ret < 0)
hid_err(sc->hdev, "usb_submit_urb failed: %d", ret);
...
}
Neither sony_suspend() nor sony_resume() seem to manage ghl_poke_timer:
drivers/hid/hid-sony.c:
static int sony_suspend(struct hid_device *hdev, pm_message_t message)
{
...
}
static int sony_resume(struct hid_device *hdev)
{
...
}
Once the system resumes, will the timer remain dead, effectively dropping
the device connection permanently? Does the suspend handler need to pause
the timer and the resume handler restart it?
[Severity: Low]
This isn't a bug introduced by this patch, but URB DMA buffers are allocated
using devm_kzalloc(), which violates DMA API cache-line alignment
requirements.
Looking at ghl_init_urb():
cr = devm_kzalloc(&sc->hdev->dev, sizeof(*cr), GFP_ATOMIC);
...
databuf = devm_kzalloc(&sc->hdev->dev, poke_size, GFP_ATOMIC);
And in sony_allocate_output_report():
sc->output_report_dmabuf =
devm_kmalloc(&sc->hdev->dev,
sizeof(union sixaxis_output_report_01),
GFP_KERNEL);
Since the devm subsystem prepends a devres_node header, the resulting
pointers are offset from ARCH_DMA_MINALIGN. Is it safe to map these unaligned
buffers for DMA by the USB core?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724142703.55813-1-doruk@0sec.ai?part=1
^ permalink raw reply
* [PATCH v2] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
From: Doruk Tan Ozturk @ 2026-07-24 14:27 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pascal Giard, linux-input, linux-kernel, stable
For GHL (Guitar Hero Live) dongles, sony_probe() arms a periodic timer:
ghl_magic_poke() (the timer callback) submits sc->ghl_urb, and the URB
completion ghl_magic_poke_cb() re-arms the timer with mod_timer().
sony_remove() drained the timer with timer_delete_sync() and then freed
the URB with usb_free_urb():
timer_delete_sync(&sc->ghl_poke_timer);
usb_free_urb(sc->ghl_urb);
timer_delete_sync() does not block re-arming, and while the URB is in
flight the timer is not pending, so the sync delete is a no-op. A URB
completion that runs after the delete re-arms the timer, and usb_free_urb()
only drops a reference -- it does not kill an in-flight URB. sc is
allocated with devm_kzalloc() and freed once sony_remove() returns, so the
re-armed ghl_poke_timer (embedded in sc) then fires on freed memory, a
use-after-free from timer softirq. This is a disconnect/rmmod race.
Poison the URB first, then shut the timer down, before freeing the URB.
usb_poison_urb() kills any in-flight URB and permanently rejects further
submissions, so a poke timer that is still pending cannot re-submit the
URB from ghl_magic_poke() in the window before timer_shutdown_sync() runs.
usb_kill_urb() would not suffice: it only cancels the in-flight URB and
leaves it submittable once it returns, so the pending timer could
re-submit it and put a fresh URB in flight over the freed sc.
timer_shutdown_sync() then drains any last callback and blocks re-arming.
The probe error path is unaffected: it is only reached before the timer
is armed.
Reproduced under KASAN on next-20260710 via dummy_hcd + raw-gadget
emulation of the GHL PS4 dongle (VID 0x1430 / PID 0x07bb): hid-sony binds
and arms the poke timer, the poke URB is held in flight, the driver is
unbound (freeing sc), then the URB is released. The completion re-arms the
timer on the freed sc, and the re-armed timer fires ~8 s later:
BUG: KASAN: slab-use-after-free in ghl_magic_poke+0x98/0xb0
Read of size 8 at addr ffff88810b02fd50 by task swapper/0/0
ghl_magic_poke+0x98/0xb0
call_timer_fn+0x35/0x2b0
__run_timers+0x69c/0x9a0
run_timer_softirq+0x173/0x2a0
Allocated by task 169: sony_probe
Freed by task 338: devres_release_group <- hid_device_remove (sony_remove)
Found by 0sec (https://0sec.ai) using automated source analysis.
Fixes: cc894ac55360 ("HID: sony: support for ghlive ps3/wii u dongles")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2: use usb_poison_urb() instead of usb_kill_urb(). usb_kill_urb() only
cancels the in-flight URB and leaves it submittable once it returns,
so a still-pending poke timer could re-submit between the kill and
timer_shutdown_sync() and race the free (Sashiko review). Inline
comment kept terse; rationale in the log.
drivers/hid/hid-sony.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 253fff4066eb..112d1deaf3f3 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2431,7 +2431,9 @@ static void sony_remove(struct hid_device *hdev)
struct sony_sc *sc = hid_get_drvdata(hdev);
if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
- timer_delete_sync(&sc->ghl_poke_timer);
+ /* poison, not kill: a pending timer must not re-submit during teardown */
+ usb_poison_urb(sc->ghl_urb);
+ timer_shutdown_sync(&sc->ghl_poke_timer);
usb_free_urb(sc->ghl_urb);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
From: sashiko-bot @ 2026-07-24 14:01 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260724134236.34304-1-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the patch fails to correctly fix the use-after-free race condition because it uses `usb_kill_urb()` instead of `usb_poison_urb()` or a reversed teardown sequence. `usb_kill_urb()` does not permanently prevent URB submissions; it allows the pending timer to submit the URB immediately after it returns.
--
--- Patch [1]: [PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind ---
commit 922f81b4c171a572e240262070ec65197e21cd83
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
This commit changes the driver teardown sequence for Guitar Hero Live
dongles to fix a use-after-free during driver unbind. It attempts to kill
the in-flight URB before shutting down the timer to prevent the URB
completion from re-arming the timer on freed memory.
Fixes: cc894ac55360 ("HID: sony: support for ghlive ps3/wii u dongles")
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index e75246d29e16d..b7b9b705b9e1c 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -2424,7 +2424,19 @@ static void sony_remove(struct hid_device *hdev)
> struct sony_sc *sc = hid_get_drvdata(hdev);
>
> if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
> - timer_delete_sync(&sc->ghl_poke_timer);
> + /*
> + * ghl_magic_poke() resubmits sc->ghl_urb from the poke timer
> + * and its completion ghl_magic_poke_cb() re-arms the timer, so
> + * an in-flight URB can re-arm the timer after a plain
> + * timer_delete_sync() (a no-op while the URB is in flight and
> + * the timer is not pending). sc is devm-allocated and freed
> + * once sony_remove() returns, so the re-armed timer would fire
> + * on freed memory. Kill the URB first so no completion can
> + * re-arm the timer, then timer_shutdown_sync() to drain it and
> + * permanently block further re-arming.
> + */
> + usb_kill_urb(sc->ghl_urb);
> + timer_shutdown_sync(&sc->ghl_poke_timer);
> + usb_free_urb(sc->ghl_urb);
[Severity: Critical]
This is a pre-existing issue, but does this teardown order still allow a
use-after-free race condition?
Because usb_kill_urb() does not permanently prevent URB submissions, it allows
the pending timer to submit the URB immediately after it returns.
Could this sequence occur if a user unbinds the driver while the URB is not
in flight and the ghl_poke_timer is pending:
1. usb_kill_urb() executes and returns immediately, decrementing urb->reject
back to 0.
2. Before timer_shutdown_sync() executes, the timer expires and its callback
ghl_magic_poke() successfully submits the URB.
3. timer_shutdown_sync() then waits for the timer callback to finish and marks
the timer as shutdown, but the URB is now in flight.
4. The driver teardown finishes and frees sc.
5. Later, the URB completes and triggers the use-after-free by accessing the
freed sc.
Would it be safer to use usb_poison_urb() instead to permanently block
submissions?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724134236.34304-1-doruk@0sec.ai?part=1
^ permalink raw reply
* [PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind
From: Doruk Tan Ozturk @ 2026-07-24 13:42 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pascal Giard, linux-input, linux-kernel, stable
For GHL (Guitar Hero Live) dongles, sony_probe() arms a periodic timer:
ghl_magic_poke() (the timer callback) submits sc->ghl_urb, and the URB
completion ghl_magic_poke_cb() re-arms the timer with mod_timer().
sony_remove() drained the timer with timer_delete_sync() and then freed
the URB with usb_free_urb():
timer_delete_sync(&sc->ghl_poke_timer);
usb_free_urb(sc->ghl_urb);
timer_delete_sync() does not block re-arming, and while the URB is in
flight the timer is not pending, so the sync delete is a no-op. A URB
completion that runs after the delete re-arms the timer, and usb_free_urb()
only drops a reference -- it does not kill an in-flight URB. sc is
allocated with devm_kzalloc() and freed once sony_remove() returns, so the
re-armed ghl_poke_timer (embedded in sc) then fires on freed memory, a
use-after-free from timer softirq. This is a disconnect/rmmod race.
Kill the URB first so no completion can re-arm the timer (further
usb_submit_urb() from the callback then fails), then timer_shutdown_sync()
to drain any last callback and permanently block re-arming, before freeing
the URB. The probe error path is unaffected: it is only reached before the
timer is armed.
Reproduced under KASAN on next-20260710 via dummy_hcd + raw-gadget
emulation of the GHL PS4 dongle (VID 0x1430 / PID 0x07bb): hid-sony binds
and arms the poke timer, the poke URB is held in flight, the driver is
unbound (freeing sc), then the URB is released. The completion re-arms the
timer on the freed sc, and the re-armed timer fires ~8 s later:
BUG: KASAN: slab-use-after-free in ghl_magic_poke+0x98/0xb0
Read of size 8 at addr ffff88810b02fd50 by task swapper/0/0
ghl_magic_poke+0x98/0xb0
call_timer_fn+0x35/0x2b0
__run_timers+0x69c/0x9a0
run_timer_softirq+0x173/0x2a0
Allocated by task 169: sony_probe
Freed by task 338: devres_release_group <- hid_device_remove (sony_remove)
Found by 0sec (https://0sec.ai) using automated source analysis.
Fixes: cc894ac55360 ("HID: sony: support for ghlive ps3/wii u dongles")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/hid/hid-sony.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 253fff4066eb..5f9670f3910a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2431,7 +2431,19 @@ static void sony_remove(struct hid_device *hdev)
struct sony_sc *sc = hid_get_drvdata(hdev);
if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
- timer_delete_sync(&sc->ghl_poke_timer);
+ /*
+ * ghl_magic_poke() resubmits sc->ghl_urb from the poke timer
+ * and its completion ghl_magic_poke_cb() re-arms the timer, so
+ * an in-flight URB can re-arm the timer after a plain
+ * timer_delete_sync() (a no-op while the URB is in flight and
+ * the timer is not pending). sc is devm-allocated and freed
+ * once sony_remove() returns, so the re-armed timer would fire
+ * on freed memory. Kill the URB first so no completion can
+ * re-arm the timer, then timer_shutdown_sync() to drain it and
+ * permanently block further re-arming.
+ */
+ usb_kill_urb(sc->ghl_urb);
+ timer_shutdown_sync(&sc->ghl_poke_timer);
usb_free_urb(sc->ghl_urb);
}
--
2.43.0
^ permalink raw reply related
* [PATCH] HID: asus: support the Zenbook A16 (UX3607OA) keyboard
From: Konrad Dybcio @ 2026-07-24 12:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-arm-msm, Bjorn Andersson,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The ASUS Zenbook A16 (UX3607OA), a Qualcomm Snapdragon X2 Elite Extreme
(Glymur)-based laptop, carries its main keyboard on an I2C-HID device
with the ID 0B05:4B42. Its Fn/media hotkeys are emitted as vendor-page
(HID_UP_ASUSVENDOR) usages on report 0x5A.
Match the device in hid-asus with the standard I2C keyboard quirks and
add the three usage mappings observed on the hardware:
0x85 -> KEY_CAMERA (Fn+F11, camera toggle)
0x86 -> KEY_PROG1 (Fn+F12, MyASUS key)
0x5f -> KEY_PROG2 (extra programmable key)
The camera-toggle key reports its usage (0x85) together with a companion
state byte in the same array report ("5a 85 01" / "5a 85 10"). The 0x10
companion aliases the brightness-down vendor usage and spuriously dims
the panel, so add QUIRK_FILTER_CAMERA_COMPANION to zero the companion
slots for this device before input mapping. The quirk is device-gated so
no other ASUS model is affected.
Assisted-by: Claude Code:claude-opus-4-8
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/hid/hid-asus.c | 18 ++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..789546238dc7 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_FILTER_CAMERA_COMPANION BIT(15)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -460,6 +461,17 @@ static int asus_raw_event(struct hid_device *hdev,
return -1;
}
+ /*
+ * The camera-toggle key reports its vendor usage (0x85) together with a
+ * companion state byte in the same array report, e.g. "5a 85 01" and
+ * "5a 85 10" for the two toggle positions. The 0x10 companion aliases the
+ * brightness-down vendor usage and would spuriously dim the panel, so drop
+ * the companion slots and leave only the camera usage for input mapping.
+ */
+ if (drvdata->quirks & QUIRK_FILTER_CAMERA_COMPANION &&
+ report->id == FEATURE_KBD_REPORT_ID && size >= 3 && data[1] == 0x85)
+ memset(&data[2], 0, size - 2);
+
return 0;
}
@@ -1051,6 +1063,8 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0x6c: asus_map_key_clear(KEY_SLEEP); break;
case 0x7c: asus_map_key_clear(KEY_MICMUTE); break;
case 0x82: asus_map_key_clear(KEY_CAMERA); break;
+ case 0x85: asus_map_key_clear(KEY_CAMERA); break;
+ case 0x86: asus_map_key_clear(KEY_PROG1); break; /* MyASUS key */
case 0x88: asus_map_key_clear(KEY_RFKILL); break;
case 0xb5: asus_map_key_clear(KEY_CALC); break;
case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP); break;
@@ -1060,6 +1074,7 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0x7e: asus_map_key_clear(KEY_EMOJI_PICKER); break;
case 0x8b: asus_map_key_clear(KEY_PROG1); break; /* ProArt Creator Hub key */
+ case 0x5f: asus_map_key_clear(KEY_PROG2); break; /* S-shaped programmable key */
case 0x6b: asus_map_key_clear(KEY_F21); break; /* ASUS touchpad toggle */
case 0x38: asus_map_key_clear(KEY_PROG1); break; /* ROG key */
case 0xba: asus_map_key_clear(KEY_PROG2); break; /* Fn+C ASUS Splendid */
@@ -1483,6 +1498,9 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
static const struct hid_device_id asus_devices[] = {
{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD), I2C_KEYBOARD_QUIRKS},
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_I2C_ZENBOOK_KEYBOARD),
+ I2C_KEYBOARD_QUIRKS | QUIRK_FILTER_CAMERA_COMPANION },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD), I2C_TOUCHPAD_QUIRKS },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b70f719b3b07..f381fcd9a76d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -220,6 +220,7 @@
#define USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD 0x183d
#define USB_DEVICE_ID_ASUSTEK_T304_KEYBOARD 0x184a
#define USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD 0x8585
+#define USB_DEVICE_ID_ASUSTEK_I2C_ZENBOOK_KEYBOARD 0x4b42
#define USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD 0x0101
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2 0x1837
---
base-commit: 9eebf259d5352b87080d67758f483583d9e763d7
change-id: 20260724-topic-asus_keyboard-83122b29dc2a
Best regards,
--
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH v14 0/2] Lenovo Yoga Book 9 keyboard dock detection
From: Ilpo Järvinen @ 2026-07-24 11:42 UTC (permalink / raw)
To: Dave Carey
Cc: platform-driver-x86, hdegoede, W_Armin, jikos, linux-input, LKML
In-Reply-To: <20260722201659.874469-1-carvsdriver@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2937 bytes --]
On Wed, 22 Jul 2026, Dave Carey wrote:
> Changes in v14 (addressing Ilpo Järvinen's v13 review of patch 2/2):
>
> - Restore #include <linux/compiler_attributes.h> and <linux/spinlock.h>,
> which were accidentally absent from v13.
> - Restore spinlock_t lock in struct yb9_kbdock_data, spin_lock_init(),
> and spin_lock/spin_unlock around input_report_switch + input_sync
> in yb9_kbdock_report().
> - Restore pointer-out yb9_kbdock_query(d, u32 *bkbd) and removal of
> zero-initializer from struct wmi_buffer out.
> - Restore int return from yb9_kbdock_sync() and error propagation in
> yb9_kbdock_resume() and yb9_kbdock_block_probe().
Hi,
Unfortunately, this still seems to miss one non-cosmetic change (diff of
diffs, v12 vs v14):
diff --git a/drivers/platform/x86/lenovo/yb9-kbdock.c b/drivers/platform/x86/lenovo/yb9>
new file mode 100644
-index 0000000..0000000
+index 0000000..40a2566
--- /dev/null
+++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
-@@ -0,0 +1,322 @@
+@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Lenovo Yoga Book 9 keyboard-dock detection
@@ -421,8 +449,10 @@
+ /*
+ * _WED(0xEB) returns EC.BKBD directly as a 32-bit integer
+ * (LENOVO_BTKBD_EVENT WmiDataId(1) uint32 Status).
-+ * Short-buffer guard is handled by .min_event_size below.
+ */
++ if (!data || data->length < sizeof(__le32))
++ return;
++
+ u32 bkbd = le32_to_cpu(*(const __le32 *)data->data);
+
+ blocking_notifier_call_chain(&yb9_kbdock_chain_head, bkbd, NULL);
@@ -441,7 +471,6 @@
+ .id_table = yb9_kbdock_event_id_table,
+ .no_singleton = true,
+ .notify_new = yb9_kbdock_notify_new,
-+ .min_event_size = sizeof(__le32),
+};
+
+/* ------------------------------------------------------------------
> Changes in v13:
[...snip...]
> Changes in v10 (addressing Armin Wolf's v9 review of patch 2/2):
[...snip...]
> - yb9_kbdock_notify_new(): drop manual length check; use
> .min_event_size = sizeof(__le32) in struct wmi_driver instead
>
> Patch 1/2 (lenovo-ymc) is unchanged from v9.
...And there's no need to apologize. :-)
I personally keep earlier versions of the patches I've sent myself around
as files so I can easily do diff-of-diffs against the previous version. It
has saved me sooo many time from making fool of myself.
I actually normally write the patch version history bullets based on that
diff so I won't forget anything important from it. Nor hallucinate some
changes that I thought I'd made but didn't (=> back to the drawing board).
If you need the old version you can probably get them from lore archive if
you don't have them around anymore (but by not placing them cleanly into
own threads one needs to be careful to get the right ones).
--
i.
^ permalink raw reply
* Re: [PATCH 4/5] dt-bindings: input/ts/zinitix: document mode 0
From: Krzysztof Kozlowski @ 2026-07-24 9:56 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Srba, Linus Walleij, Peter Griffin, Alim Akhtar,
linux-input, linux-kernel, devicetree, linux-arm-kernel,
linux-samsung-soc
In-Reply-To: <20260724-zinitix-modes-v1-4-f6df7d86a878@disroot.org>
On Fri, Jul 24, 2026 at 12:54:06AM +0530, Kaustabh Chakraborty wrote:
> Zinitix touchscreens support three modes, numbered 0 to 2. These modes
> differ based on their event report schema.
>
> The devicetree schema does not mention mode 0, thus does not consider it
> as a valid state. Add it.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> .../devicetree/bindings/input/touchscreen/zinitix,bt400.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] HID: corsair: fix use-after-free by reordering remove sequence
From: sashiko-bot @ 2026-07-24 9:37 UTC (permalink / raw)
To: Chen Changcheng; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260724091906.240588-1-chenchangcheng@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Pre-existing Use-After-Free race condition in k90_record_led_work and k90_backlight_work.
--
--- Patch [1]: [PATCH] HID: corsair: fix use-after-free by reordering remove sequence ---
commit 79119529f5e0e6e15e9750d9ceb51ce25886fb73
Author: Chen Changcheng <chenchangcheng@kylinos.cn>
Subject: HID: corsair: fix use-after-free by reordering remove sequence
This commit fixes a use-after-free in the corsair driver during device
removal. It reorders the remove sequence to call hid_hw_stop() before
freeing structures, preventing concurrent USB events from accessing freed
memory. It also sets drvdata->k90 to NULL after freeing it as a defensive
measure.
> diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
> index 21cd8b12a7572..ac12877a6db7b 100644
> --- a/drivers/hid/hid-corsair.c
> +++ b/drivers/hid/hid-corsair.c
[ ... ]
> @@ -596,10 +597,10 @@ static int corsair_probe(struct hid_device *dev, const struct hid_device_id *id)
>
> static void corsair_remove(struct hid_device *dev)
> {
> + hid_hw_stop(dev);
> +
> k90_cleanup_macro_functions(dev);
> k90_cleanup_backlight(dev);
[Severity: High]
This is a pre-existing issue, but is there a use-after-free race between
these cleanup functions and the LED worker threads?
If a user modifies the LED brightness via sysfs, it schedules
k90_backlight_work() or k90_record_led_work(). If the device is unplugged
concurrently, they can race:
CPU1 (Worker)
k90_backlight_work() {
if (led->removed)
return;
<preempted>
CPU2 (Remove)
corsair_remove() {
k90_cleanup_backlight() {
drvdata->backlight->removed = true;
led_classdev_unregister(&drvdata->backlight->cdev);
// This unregisters and can free led->cdev.dev
cancel_work_sync(&drvdata->backlight->work);
// Blocks waiting for CPU1
}
}
CPU1 (Worker resumes)
dev = led->cdev.dev->parent;
// UAF dereferencing the freed led->cdev.dev
Would swapping the order of led_classdev_unregister() and cancel_work_sync()
in both cleanup functions prevent this memory corruption?
> -
> - hid_hw_stop(dev);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724091906.240588-1-chenchangcheng@kylinos.cn?part=1
^ permalink raw reply
* [PATCH] HID: corsair: fix use-after-free by reordering remove sequence
From: Chen Changcheng @ 2026-07-24 9:19 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, Chen Changcheng
On device removal corsair_remove() currently does:
k90_cleanup_macro_functions() ─┐
kfree(k90) │ k90 freed, drvdata->k90
│ is now a dangling pointer
k90_cleanup_backlight() │
hid_hw_stop(dev) ─┘ HID I/O finally stopped
The problem is that kfree happens before hid_hw_stop. Between them,
corsair_event() can still be invoked on another CPU:
CPU 0 (remove path) CPU 1 (USB interrupt)
────────────────── ────────────────────
kfree(k90)
corsair_event()
↓ if (!drvdata->k90)
↓ return 0; ← non-NULL!
↓ drvdata->k90->record_led
↓ .brightness = x;
↓ ^^^^^^^^^^^^^^^^^^^^^^^^
↓ UAF write into freed slab
hid_hw_stop(dev)
hid_disconnect()
clear claimed flags
usbhid_stop()
kill URBs
The NULL check in corsair_event() is ineffective because
k90_cleanup_macro_functions() never clears drvdata->k90 after kfree.
Fix by reordering so hid_hw_stop() runs first, eliminating the window:
hid_hw_stop(dev) ─┐ HID I/O stopped first,
k90_cleanup_macro_functions() │ no more events can arrive
k90_cleanup_backlight() ─┘
Set drvdata->k90 = NULL after kfree() as a defensive measure, matching
the existing pattern in k90_init_macro_functions()'s error path.
Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
drivers/hid/hid-corsair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 21cd8b12a757..ac12877a6db7 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -545,6 +545,7 @@ static void k90_cleanup_macro_functions(struct hid_device *dev)
kfree(k90->record_led.cdev.name);
kfree(k90);
+ drvdata->k90 = NULL;
}
}
@@ -596,10 +597,10 @@ static int corsair_probe(struct hid_device *dev, const struct hid_device_id *id)
static void corsair_remove(struct hid_device *dev)
{
+ hid_hw_stop(dev);
+
k90_cleanup_macro_functions(dev);
k90_cleanup_backlight(dev);
-
- hid_hw_stop(dev);
}
static int corsair_event(struct hid_device *dev, struct hid_field *field,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] Input: uinput/uhid - disallow control characters in phys paths
From: David Rheinsberg @ 2026-07-24 7:51 UTC (permalink / raw)
To: Peter Hutterer, Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov
Cc: linux-input, linux-kernel
In-Reply-To: <20260717-wip-uinput-sanitize-phys-v1-1-f6459542ce4b@who-t.net>
Hi Peter!
On Fri, Jul 17, 2026, at 6:49 AM, Peter Hutterer wrote:
> There is no good reason to support those, no physical device will ever
> produce those. Allowing \n in phys previously triggered CVE-2026-50292
> in libinput - there the PHYS udev property value was used as part of
> another udev property value. The linebreak then caused the property
> to be split across two lines, allowing uinput devices to inject
> malicious properties. While the bug is squarely inlibinput's court
> there still isn't a good reason for control characters in uinput/uhid.
>
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> drivers/hid/uhid.c | 1 +
> drivers/input/misc/uinput.c | 1 +
> include/linux/input.h | 15 +++++++++++++++
> 3 files changed, 17 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 37b60c3aaf66..baf1fe8290f7 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -513,16 +513,17 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> ret = PTR_ERR(hid);
> goto err_free;
> }
>
> BUILD_BUG_ON(sizeof(hid->name) != sizeof(ev->u.create2.name));
> strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
> BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
> strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
> + input_sanitize_phys(hid->phys);
> BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
> strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
>
> hid->ll_driver = &uhid_hid_driver;
> hid->bus = ev->u.create2.bus;
> hid->vendor = ev->u.create2.vendor;
> hid->product = ev->u.create2.product;
> hid->version = ev->u.create2.version;
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index d32fa4b508fc..70fe4f3e73bf 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -998,16 +998,17 @@ static long uinput_ioctl_handler(struct file
> *file, unsigned int cmd,
>
> phys = strndup_user(p, 1024);
> if (IS_ERR(phys)) {
> retval = PTR_ERR(phys);
> goto out;
> }
>
> kfree(udev->dev->phys);
> + input_sanitize_phys(phys);
> udev->dev->phys = phys;
> goto out;
>
> case UI_BEGIN_FF_UPLOAD:
> retval = uinput_ff_upload_from_user(p, &ff_up);
> if (retval)
> goto out;
>
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 76f7aa226202..6c182f5c783f 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -527,16 +527,31 @@ int input_set_keycode(struct input_dev *dev,
>
> bool input_match_device_id(const struct input_dev *dev,
> const struct input_device_id *id);
>
> void input_enable_softrepeat(struct input_dev *dev, int delay, int period);
>
> bool input_device_enabled(struct input_dev *dev);
>
> +/**
> + * input_sanitize_phys - replace invalid characters in a phys string
> + * @phys: the phys path to sanitize (modified in place)
> + *
> + * Replaces any control characters and non-ASCII characters with '?'.
> + **/
> +static inline void input_sanitize_phys(char *phys)
> +{
> + char *p;
> +
> + for (p = phys; *p; p++)
> + if (*p < 0x20 || *p > 0x7e)
> + *p = '?';
> +}
> +
Reviewed-by: David Rheinsberg <david@readahead.eu>
I would also be fine to just reject them in uinput, but I guess this is the less intrusive option.
Thanks a lot!
David
> extern const struct class input_class;
>
> /**
> * struct ff_device - force-feedback part of an input device
> * @upload: Called to upload an new effect into device
> * @erase: Called to erase an effect from device
> * @playback: Called to request device to start playing specified effect
> * @set_gain: Called to set specified gain
>
> ---
> base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
> change-id: 20260717-wip-uinput-sanitize-phys-abb6cf40e577
>
> Best regards,
> --
> Peter Hutterer <peter.hutterer@who-t.net>
^ permalink raw reply
* Re: [PATCH 05/12] gpio: adp5585: use devm_blocking_notifier_chain_register()
From: Bartosz Golaszewski @ 2026-07-24 7:47 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, jic23, dlechner, nuno.sa, andy, dmitry.torokhov,
hansg, ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco,
kaihengf, ankita, leitao, jason, pedro.pbg, paulmck, frederic,
kees, linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175513.39849-1-farbere@amazon.com>
On Thu, 23 Jul 2026 19:55:06 +0200, Eliav Farber <farbere@amazon.com> said:
> Replace the blocking_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_blocking_notifier_chain_register(), removing the
> adp5585_gpio_unreg_notifier() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 11/12] gpio: gpiolib-kunit: use devm_blocking_notifier_chain_register()
From: Bartosz Golaszewski @ 2026-07-24 7:47 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, jic23, dlechner, nuno.sa, andy, dmitry.torokhov,
hansg, ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco,
kaihengf, ankita, leitao, jason, pedro.pbg, paulmck, frederic,
kees, linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175632.44812-2-farbere@amazon.com>
On Thu, 23 Jul 2026 19:56:31 +0200, Eliav Farber <farbere@amazon.com> said:
> Replace the blocking_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_blocking_notifier_chain_register(), removing the
> gpio_unbind_unregister_notifier() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
This could conflict with the changes I already have queued so either it waits
until the next cycle or I'd need an immutable branch with patch 1/12.
Bartosz
^ permalink raw reply
* Re: [PATCH 10/12] gpio: eic-sprd: use devm_atomic_notifier_chain_register()
From: Bartosz Golaszewski @ 2026-07-24 7:46 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, jic23, dlechner, nuno.sa, andy, dmitry.torokhov,
hansg, ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco,
kaihengf, ankita, leitao, jason, pedro.pbg, paulmck, frederic,
kees, linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175632.44812-1-farbere@amazon.com>
On Thu, 23 Jul 2026 19:56:30 +0200, Eliav Farber <farbere@amazon.com> said:
> Replace the atomic_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_atomic_notifier_chain_register(), removing the
> sprd_eic_unregister_notifier() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 00/12] notifier: add device-managed registration APIs and convert drivers
From: Bartosz Golaszewski @ 2026-07-24 7:46 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, jic23, dlechner, nuno.sa, andy, dmitry.torokhov,
hansg, ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco,
kaihengf, ankita, leitao, jason, pedro.pbg, paulmck, frederic,
kees, linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175325.37397-1-farbere@amazon.com>
On Thu, 23 Jul 2026 19:53:13 +0200, Eliav Farber <farbere@amazon.com> said:
> Many drivers repeat the same boilerplate when registering notifiers with
> device lifetime:
>
> 1. Register the notifier with *_notifier_chain_register()
> 2. Check for error
> 3. Register a devm action to unregister on teardown
> 4. Implement a per-driver static unregister callback
>
> This series adds devm_atomic_notifier_chain_register(),
> devm_blocking_notifier_chain_register(), and
> devm_raw_notifier_chain_register() that automatically unregister the
> notifier when the device is unbound, then converts 11 drivers to use
> them.
>
> Each conversion eliminates a per-driver unregister callback and the
> associated devm_add_action_or_reset() call, reducing code by ~15 lines
> per driver.
>
> The implementation follows the established devres pattern used by other
> device-managed kernel APIs.
>
> Checkpatch reports 6 CHECKs (0 errors, 0 warnings) on patch 1:
> - "extern prototypes should be avoided in .h files"
> - "Alignment should match open parenthesis"
>
> Both are intentional to match the existing coding style in notifier.h,
> where all declarations use 'extern' and tab-indented continuation lines.
>
What is the merging strategy? Acks and all patches go through driver core?
Bartosz
^ permalink raw reply
* Re: [PATCH 01/12] notifier: add device-managed registration APIs
From: Bartosz Golaszewski @ 2026-07-24 7:45 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, jic23, dlechner, nuno.sa, andy, dmitry.torokhov,
hansg, ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco,
kaihengf, ankita, leitao, jason, pedro.pbg, paulmck, frederic,
kees, linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175325.37397-2-farbere@amazon.com>
On Thu, 23 Jul 2026 19:53:14 +0200, Eliav Farber <farbere@amazon.com> said:
> Add devm_atomic_notifier_chain_register(),
> devm_blocking_notifier_chain_register(), and
> devm_raw_notifier_chain_register() that automatically unregister the
> notifier when the device is unbound.
>
> Many drivers repeat the same boilerplate pattern:
>
> 1. Register the notifier with *_notifier_chain_register()
> 2. Check for error
> 3. Register a devm action to unregister on teardown
> 4. Implement a per-driver static unregister callback
>
> With the new devm_*_notifier_chain_register() APIs, this reduces to a
> single call with one error path, eliminating per-driver unregister
> callbacks entirely.
>
> The implementation follows the established devres pattern used by other
> device-managed kernel APIs.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> include/linux/notifier.h | 10 +++
> kernel/notifier.c | 151 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 161 insertions(+)
>
> diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> index 01b6c9d9956f..aa3745d7459b 100644
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -46,6 +46,7 @@
> * often but notifier_blocks will seldom be removed.
> */
>
> +struct device;
> struct notifier_block;
>
> typedef int (*notifier_fn_t)(struct notifier_block *nb,
> @@ -145,10 +146,19 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
>
> extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> struct notifier_block *nb);
> +extern int devm_atomic_notifier_chain_register(struct device *dev,
> + struct atomic_notifier_head *nh,
> + struct notifier_block *nb);
Please don't use extern with function prototypes in new code.
> extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
> struct notifier_block *nb);
> +extern int devm_blocking_notifier_chain_register(struct device *dev,
> + struct blocking_notifier_head *nh,
> + struct notifier_block *nb);
> extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
> struct notifier_block *nb);
> +extern int devm_raw_notifier_chain_register(struct device *dev,
> + struct raw_notifier_head *nh,
> + struct notifier_block *nb);
> extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
> struct notifier_block *nb);
>
> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index 2f9fe7c30287..b811638aa878 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -1,4 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/device.h>
> #include <linux/kdebug.h>
> #include <linux/kprobes.h>
> #include <linux/export.h>
> @@ -197,6 +198,55 @@ int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
> }
> EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
>
> +struct atomic_notifier_chain_devres {
> + struct atomic_notifier_head *nh;
> + struct notifier_block *nb;
> +};
> +
> +static void devm_atomic_notifier_chain_unregister(struct device *dev, void *res)
> +{
> + struct atomic_notifier_chain_devres *dr = res;
> +
> + atomic_notifier_chain_unregister(dr->nh, dr->nb);
> +}
> +
> +/**
> + * devm_atomic_notifier_chain_register - Device-managed atomic notifier registration
> + * @dev: Device to tie the notifier lifetime to
> + * @nh: Pointer to head of the atomic notifier chain
> + * @n: New entry in notifier chain
> + *
> + * Adds a notifier to an atomic notifier chain and registers a cleanup
> + * action to automatically unregister it when @dev is unbound.
> + *
> + * Returns 0 on success, negative errno on error.
Should be:
Returns:
0 on success...
Bart
^ permalink raw reply
* hid-sensor-custom: async notification for input-report sysfs attributes? (human presence / attention sensors)
From: Daniel Paul Perinchery @ 2026-07-24 7:12 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Benjamin Tissoires
Cc: linux-input, linux-iio, linux-kernel
Hi,
I've been investigating why userspace can't efficiently watch a
hid-sensor-custom input-report value (specifically a human-presence
sensor, HID usage 2000e1, on an AMD Sensor Fusion Hub / amd_sfh
platform) for changes without polling, and wanted to check whether
there's interest in closing this gap before I try to write anything.
Setup: Lenovo IdeaPad Pro 5 14AKP10, AMD Ryzen AI 7 350, kernel 7.0.13
(Debian/Parrot, but this is stock hid-sensor-custom.c). Presence and
gaze/attention are exposed as separate logical sensors under the same
physical hub (HID-SENSOR-2000e1.x.auto), each with the usual
auto-generated input-N-<usage>-value sysfs attribute.
What I found tracing show_value() in hid-sensor-custom.c: for input
reports, it calls sensor_hub_input_attr_get_raw_value(..., report_id,
SENSOR_HUB_SYNC, false) -- i.e. every read of the -value file issues a
synchronous GET_REPORT to the hub and returns the live answer. There's
no cached field backing that attribute, so there's nothing for a
notify-on-change mechanism to attach to on that path.
Separately, hid_sensor_capture_sample() *is* a genuine async callback
fired on hub-pushed input reports, but as far as I can tell it only
feeds the raw kfifo behind the driver's misc-device streaming
interface (custom_dev), gated on test_bit(0,
&sensor_inst->misc_opened). It doesn't appear to update any per-field
cached value, so it isn't reachable from the sysfs show_value() path
either.
Net effect: there's no way for a userspace consumer to poll()/
select() on the -value file and be woken only on real hardware events.
The hub itself is already configured for threshold-based reporting
(property-reporting-state = 2, i.e. "Threshold Events") in my case, so
the inefficiency is entirely on the kernel/sysfs side -- userspace has
to fall back to timed re-reads of a synchronous, firmware-round-trip
attribute, which is more expensive per-read than a typical cached
sysfs value and awkward to size a safe polling interval for.
This seems like a real gap for the presence/proximity use case
specifically (walk-away lock, auto-dim, etc.), which is inherently
event-oriented and is exactly the kind of feature vendors implement
natively on Windows via push notifications from the same class of
sensor.
Before I put together a patch I wanted to ask: is there interest in
adding async caching + sysfs_notify() (or sysfs_notify_dirent(), if
attrs end up carrying a stored kernfs_node) for input-report values
specifically, sourced from hid_sensor_capture_sample()'s existing
async path? Rough shape as I see it:
- add a cached "last input value" (+ a bool "valid") per relevant
field in struct hid_sensor_custom_field
- in hid_sensor_capture_sample(), in addition to (or instead of,
behind a config/attr) feeding the misc-device fifo, decode and store
the value for the matching field, then sysfs_notify() its -value
attribute
- leave show_value()'s SENSOR_HUB_SYNC path untouched for feature
reports and for any input attrs that never get an async report, so
nothing regresses for existing consumers
Happy to be told this already exists via a different interface I've
missed (I did look at whether this should just go through the IIO
buffered/trigger interface instead, given some other hid-sensor-*
drivers are IIO-backed -- but hid-sensor-custom exposes vendor/custom
usages directly via this sysfs shape rather than IIO channels, so I
wasn't sure that's the right layer to add it at). Also open to being
told the misc-device/fifo interface is meant to be the async
consumption path already and userspace tools should be using that
instead of polling -value -- if so, is there existing tooling/docs for
that path I should be pointing presence-detection use cases at?
Not attaching a patch yet since I'd rather get a read on the right
approach (and whether it's wanted at all) before writing one.
Thanks,
Daniel Paul
^ permalink raw reply
* Re: [PATCH v4 4/4] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Krzysztof Kozlowski @ 2026-07-24 6:06 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, Liam Girdwood, linux-input,
linux-kernel, linux-leds, linux-omap, Mark Brown, Mark Jackson,
Pavel Machek, Rob Herring, Roger Quadros, Tony Lindgren,
daniel.baluta, simona.toaca, goledhruva, m-chawdhry
In-Reply-To: <20260723100605.628882-5-egbostina@gmail.com>
On Thu, Jul 23, 2026 at 10:06:02AM +0000, Eduard Bostina wrote:
> Convert the Texas Instruments TPS65217 bindings to DT schema.
>
> During the conversion, the following updates were made:
> - Documented the 'regulators', 'charger' and 'pwrbutton' child nodes,
> which are used by the boards but were missing from the old txt
> binding.
> - Documented the 'interrupts', 'interrupt-controller', '#interrupt-cells'
> and 'ti,pmic-shutdown-controller' properties, which are used by the
> am335x boards and read by the driver.
>
> The regulator/tps65217.txt binding documented the same 'ti,tps65217'
> compatible and is superseded by this schema, so remove it as well.
>
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* [syzbot] Monthly input report (Jul 2026)
From: syzbot @ 2026-07-24 4:32 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 2 new issues were detected and 0 were fixed.
In total, 19 issues are still open and 63 have already been fixed.
There are also 3 low-priority issues.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 987 Yes KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
<2> 245 Yes WARNING in cm109_input_open/usb_submit_urb (3)
https://syzkaller.appspot.com/bug?extid=ac0f9c4cc1e034160492
<3> 147 No KASAN: slab-use-after-free Read in report_descriptor_read
https://syzkaller.appspot.com/bug?extid=bc537ca7a0efe33988eb
<4> 36 Yes INFO: task hung in devres_release_group (2)
https://syzkaller.appspot.com/bug?extid=d789904ff97c2f3dac88
<5> 23 Yes INFO: task hung in __input_unregister_device (5)
https://syzkaller.appspot.com/bug?extid=78e2288f58b881ed3c45
<6> 5 No possible deadlock in tasklet_action_common (2)
https://syzkaller.appspot.com/bug?extid=b5d7ab56d43de3fd5aac
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH 08/12] ACPI: APEI: GHES: use devm_blocking_notifier_chain_register()
From: Jonathan Cameron @ 2026-07-24 0:50 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, dlechner, nuno.sa, andy, dmitry.torokhov, hansg,
ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco, kaihengf,
ankita, leitao, jason, pedro.pbg, paulmck, frederic, kees,
linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175513.39849-4-farbere@amazon.com>
On Thu, 23 Jul 2026 17:55:09 +0000
Eliav Farber <farbere@amazon.com> wrote:
> Replace the blocking_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_blocking_notifier_chain_register(), removing the
> ghes_vendor_record_notifier_destroy() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> drivers/acpi/apei/ghes.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 3236a3ce79d6..3aa29dcd70da 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -689,21 +689,12 @@ void ghes_unregister_vendor_record_noti fier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL_GPL(ghes_unregister_vendor_record_notifier);
>
> -static void ghes_vendor_record_notifier_destroy(void *nb)
> -{
> - ghes_unregister_vendor_record_notifier(nb);
> -}
> -
> int devm_ghes_register_vendor_record_notifier(struct device *dev,
> struct notifier_block *nb)
> {
> - int ret;
> -
> - ret = ghes_register_vendor_record_notifier(nb);
> - if (ret)
> - return ret;
> -
> - return devm_add_action_or_reset(dev, ghes_vendor_record_notifier_destroy, nb);
> + return devm_blocking_notifier_chain_register(dev,
> + &vendor_record_notify_list,
> + nb);
Whilst correct, I'm less keen on this one because it looses the obvious connection
to the non devm form. I suppose with the code being so near
to the non devm we can go with this.
For that matter why do we have the non devm variants? There are no users in tree.
Can we just rip that and the unregister given this is removing the only users?
Thanks,
Jonathan
> }
> EXPORT_SYMBOL_GPL(devm_ghes_register_vendor_record_notifier);
>
^ permalink raw reply
* Re: [PATCH 04/12] iio: position: iqs624: use devm_blocking_notifier_chain_register()
From: Jonathan Cameron @ 2026-07-24 0:43 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, dlechner, nuno.sa, andy, dmitry.torokhov, hansg,
ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco, kaihengf,
ankita, leitao, jason, pedro.pbg, paulmck, frederic, kees,
linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175325.37397-5-farbere@amazon.com>
On Thu, 23 Jul 2026 17:53:17 +0000
Eliav Farber <farbere@amazon.com> wrote:
> Replace the blocking_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_blocking_notifier_chain_register(), removing the
> iqs624_pos_notifier_unregister() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
Acked-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 03/12] iio: light: iqs621-als: use devm_blocking_notifier_chain_register()
From: Jonathan Cameron @ 2026-07-24 0:43 UTC (permalink / raw)
To: Eliav Farber
Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, lenb,
laurent.pinchart, linusw, brgl, orsonzhai, baolin.wang,
zhang.lyra, dlechner, nuno.sa, andy, dmitry.torokhov, hansg,
ilpo.jarvinen, W_Armin, ukleinek, fabio.m.de.francesco, kaihengf,
ankita, leitao, jason, pedro.pbg, paulmck, frederic, kees,
linux-acpi, linux-kernel, linux-gpio, linux-pwm, linux-iio,
linux-input, platform-driver-x86
In-Reply-To: <20260723175325.37397-4-farbere@amazon.com>
On Thu, 23 Jul 2026 17:53:16 +0000
Eliav Farber <farbere@amazon.com> wrote:
> Replace the blocking_notifier_chain_register() +
> devm_add_action_or_reset() pattern with a single call to
> devm_blocking_notifier_chain_register(), removing the
> iqs621_als_notifier_unregister() callback.
>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
Acked-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Seems relatively unlikely to cause a merge conflict if these changes
go through non subsystem trees - hence the Ack rather than asking
for an immutable branch with patch 1 on it.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH] HID: multitouch: stop the release timer from being rearmed on remove
From: sashiko-bot @ 2026-07-24 0:24 UTC (permalink / raw)
To: Aldo Ariel Panzardo; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260724000958.938675-1-qwe.aldo@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Out-of-bounds write via set_bit() leading to memory corruption by malicious HID devices.
- [High] Use-After-Free of report buffers in hid_haptic_destroy during device teardown.
- [High] Race condition during probe leading to NULL pointer dereference in mt_sync_frame.
- [High] Sleeping in atomic context when processing unmapped HID reports.
- [Medium] Data race modifying td->applications list without synchronization.
--
--- Patch [1]: [PATCH] HID: multitouch: stop the release timer from being rearmed on remove ---
commit b618d8ab09924436537bc4c4c724c79da532672e
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Subject: HID: multitouch: stop the release timer from being rearmed on remove
mt_remove() quiesces the sticky-finger timer before stopping the
hardware. timer_delete_sync() waits for a running callback and
dequeues the timer, but it does not stop the timer from being armed
again. Use timer_shutdown_sync() instead.
Cc: stable@vger.kernel.org
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260723224211.613112-1-you@example.com?part=1
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 0495152091e3b..f25065b9ec66e 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -2233,7 +2233,7 @@ static void mt_remove(struct hid_device *hdev)
> {
> struct mt_device *td = hid_get_drvdata(hdev);
>
> - timer_delete_sync(&td->release_timer);
> + timer_shutdown_sync(&td->release_timer);
>
> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
> hid_hw_stop(hdev);
> }
[ ... ]
drivers/hid/hid-haptic.c:hid_haptic_destroy() {
...
kfree(haptic->stop_effect.report_buf);
haptic->stop_effect.report_buf = NULL;
if (haptic->effect) {
for (r = 0; r < ff->max_effects; r++)
kfree(haptic->effect[r].report_buf);
kfree(haptic->effect);
}
haptic->effect = NULL;
destroy_workqueue(haptic->wq);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a
use-after-free during device teardown?
When an input device is destroyed, input_ff_flush() queues
stop_effect.work on the workqueue. If the work is still pending,
destroy_workqueue() will wait for it to finish. However, the report
buffers are freed before the workqueue is drained.
When haptic_work_handler() executes, it calls play_effect() which uses
the freed effect->report_buf.
Should destroy_workqueue() be called before freeing the report buffers?
drivers/hid/hid-multitouch.c:mt_probe() {
...
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret)
return ret;
...
if (td->is_haptic_touchpad) {
if (hid_haptic_init(hdev, &td->haptic)) {
dev_warn(&hdev->dev, "Cannot allocate haptic for %s\n",
hdev->name);
td->is_haptic_touchpad = false;
devm_kfree(&hdev->dev, td->haptic);
}
...
}
[Severity: High]
This is a pre-existing issue, but does this create a race condition if
hid_haptic_init() fails?
Since hid_hw_start() enables hardware interrupts earlier in mt_probe(),
an interrupt could arrive after hid_haptic_init() fails (setting td->haptic
to NULL) but before td->is_haptic_touchpad is set to false.
If mt_sync_frame() runs in this window, it will see td->is_haptic_touchpad
as true and call hid_haptic_pressure_reset(td->haptic), dereferencing a
NULL pointer.
Should the interrupt enablement or error handling order be adjusted?
drivers/hid/hid-multitouch.c:struct mt_device {
...
unsigned long mt_io_flags;
__u8 inputmode_value;
...
struct list_head applications;
...
}
drivers/hid/hid-multitouch.c:mt_process_slot() {
...
set_bit(slotnum, &td->mt_io_flags);
...
}
[Severity: Critical]
This is a pre-existing issue, but does this set_bit() call allow an
out-of-bounds write?
A malicious USB HID device could declare a contact max up to 250, resulting
in a slotnum up to 249. Since mt_io_flags is a single unsigned long, calling
set_bit(249, ...) overflows the variable by up to 31 bytes.
This would corrupt adjacent fields in the mt_device struct, including the
inputmode_value and the applications list pointers.
Can the slot index be constrained or the storage for mt_io_flags be expanded?
drivers/hid/hid-multitouch.c:mt_allocate_report_data() {
...
rdata = devm_kzalloc(&td->hdev->dev, sizeof(*rdata), GFP_KERNEL);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but can this sleep in atomic
context?
If a device sends an unmapped report ID dynamically over the interrupt
endpoint, the URB completion handler runs in atomic context and calls
hid_report_raw_event(), which eventually calls mt_report().
mt_report() then calls mt_find_report_data() and mt_allocate_report_data().
Since devm_kzalloc() is called with GFP_KERNEL here, it could fault when
processing these unmapped reports in atomic context.
Should this use GFP_ATOMIC instead?
drivers/hid/hid-multitouch.c:mt_set_quirks() {
...
list_for_each_entry(application, &td->applications, list) {
...
}
drivers/hid/hid-multitouch.c:mt_allocate_application() {
...
list_add_tail(&mt_application->list, &td->applications);
...
}
[Severity: Medium]
This is a pre-existing issue, but is there a data race on the
td->applications list?
mt_set_quirks() iterates over td->applications without locking.
Concurrently, if the device receives an unmapped report, mt_report() can
call mt_allocate_application() from the interrupt path, which adds to the
same list using list_add_tail().
Could this concurrent modification without synchronization corrupt the
linked list?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724000958.938675-1-qwe.aldo@gmail.com?part=1
^ permalink raw reply
page: next (older)
- 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