* Re: [PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()
2026-06-12 15:48 [PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event() Jiri Kosina
@ 2026-06-12 16:04 ` sashiko-bot
2026-06-12 21:30 ` Jiri Kosina
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-06-12 16:04 UTC (permalink / raw)
To: Jiri Kosina; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] TOCTOU race condition causing Use-After-Free during hidpp_connect_event error cleanup.
- [High] Use-After-Free due to dangling hidpp->input if hidinput_connect() fails during initial probe.
- [High] NULL pointer dereference in input_ff_upload if hidpp_ff_init fails midway.
--
commit 0f581c2f8ce1a26d47a4afafa4a00600b3d649dd
Author: Jiri Kosina <jikos@kernel.org>
HID: hidpp: fix potential UAF in hidpp_connect_event()
If input_register_device() fails, we call input_free_device(), but keep
stale pointer to the old device in hidpp->input, which could potentially
lead to UAF. Fix that by resetting it to NULL before returning from
hidpp_connect_event().
Reported-by: zdi-disclosures@trendmicro.com
Signed-off-by: Jiri Kosina <jkosina@suse.com>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index ccbf28869a968..2800c31e54e41 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4296,6 +4296,7 @@ static void hidpp_connect_event(struct work_struct *work)
> ret = input_register_device(input);
> if (ret) {
> input_free_device(input);
> + hidpp->input = NULL;
[Severity: High]
This is a pre-existing issue, but does setting this pointer to NULL after
freeing the input device leave a race window?
Because hidpp_connect_event() runs asynchronously in a workqueue, incoming
HID reports can be processed concurrently by hidpp_raw_event() in
interrupt context.
Could an interrupt arrive right after input_free_device() but before the
pointer is cleared, causing hidpp_raw_event() to bypass the
"if (!hidpp->input)" check and dereference the freed memory?
Would it be safer to clear the pointer before calling input_free_device(),
or does this require further synchronization since hidpp_raw_event()
accesses the pointer without holding locks?
[Severity: High]
This is a pre-existing issue, but does a similar use-after-free exist
during the initial probe path?
In hidpp_input_configured(), hidpp->input is persistently set before the
HID core attempts to register the input device:
drivers/hid/hid-logitech-hidpp.c:hidpp_input_configured() {
...
hidpp_populate_input(hidpp, input);
...
}
If input_register_device() subsequently fails inside hidinput_connect(),
the input core destroys the input_dev object, but hidpp->input is never
cleared.
Since hid_connect() can still succeed if other interfaces connect
successfully, incoming HID reports routed to hidpp_raw_event()
would then dereference the dangling hidpp->input pointer.
Could this lead to a use-after-free?
[Severity: High]
This is a pre-existing issue, but is there a potential NULL pointer
dereference if hidpp_ff_init() fails midway?
In hidpp_ff_init(), input_ff_create() is called early, which allocates
dev->ff and enables the EV_FF bit. If a subsequent memory allocation fails:
drivers/hid/hid-logitech-hidpp.c:hidpp_ff_init() {
...
data = kmemdup(data, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
...
}
The function returns without calling input_ff_destroy(). Later,
hidpp_probe() explicitly ignores this error:
drivers/hid/hid-logitech-hidpp.c:hidpp_probe() {
...
if (ret) {
hid_warn(hidpp->hid_dev,
"Unable to initialize force feedback support, errno %d\n",
ret);
ret = 0;
}
...
}
This allows the broken device to be registered. If userspace then sends an
EVIOCSFF ioctl, the input core invokes input_ff_upload(), which
unconditionally calls the driver's ff->upload function.
Since ff->upload was never assigned due to the early failure, could
dereferencing this NULL pointer cause a crash?
> return;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/r7qq6043-p432-51o0-3s93-r9382q44n027@xreary.bet?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread