[PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.com>

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>
---

v1->v2: set hidpp->input to NULL before freeing the device to avoid race 
as reported by Sashiko

 drivers/hid/hid-logitech-hidpp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ccbf28869a96..d8e86b6ccf37 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4295,6 +4295,7 @@ static void hidpp_connect_event(struct work_struct *work)
 
 	ret = input_register_device(input);
 	if (ret) {
+		hidpp->input = NULL;
 		input_free_device(input);
 		return;
 	}
-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Dangling pointer Use-After-Free when `input_register_device()` fails during synchronous probe via `hid_connect()`.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race condition leading to a Use-After-Free.
--

commit 958cf4bb8a3ce7336c278ac4fb346b928f5fe9b9
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..d8e86b6ccf375 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4295,6 +4295,7 @@ static void hidpp_connect_event(struct work_struct *work)
>  
>  	ret = input_register_device(input);
>  	if (ret) {
> +		hidpp->input = NULL;

[Severity: High]
This is a pre-existing issue, but does this clear the shared state too late to
prevent a race condition?

Since hidpp_connect_event() runs asynchronously in a workqueue, hardware
events can be processed concurrently in interrupt context. The allocated
device pointer is published to the global hidpp->input state via
hidpp_populate_input() before the registration attempt.

A concurrent raw event could read hidpp->input and pass the safety check:

drivers/hid/hid-logitech-hidpp.c:wtp_raw_event() {
	if (!wd || !hidpp->input)
		return 1;
}

If input_register_device() then fails, the error path clears hidpp->input
and frees the memory locklessly.

Could the concurrent raw event handler, already past the safety check,
access the freed memory and cause a use-after-free?

>  		input_free_device(input);
>  		return;

[Severity: Critical]
This is also a pre-existing issue, but does a similar dangling pointer remain
if registration fails during synchronous probe?

While this patch addresses a dangling pointer in hidpp_connect_event(), a
similar situation appears to exist in the hid core connection path. For
non-delayed devices, hidinput_connect() calls hidpp_input_configured(),
which caches hidinput->input into hidpp->input:

drivers/hid/hid-logitech-hidpp.c:hidpp_input_configured() {
	hidpp_populate_input(hidpp, input);
}

Immediately after, hidinput_connect() calls input_register_device(). If
registration fails, the error path frees the device:

drivers/hid/hid-input.c:hidinput_connect() {
		if (input_register_device(hidinput->input))
			goto out_unwind;
}

Because hid_connect() ignores the hidinput_connect() failure when the
driver has a raw_event handler, probe succeeds and the device remains active.

Would incoming hardware events then trigger hidpp_raw_event(), successfully
pass the check against hidpp->input, and dereference the freed pointer?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/qq420675-14n0-p23s-10oo-1n8113s2nq22@xreary.bet?part=1

[PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.com>

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 1990ba5b26ea..e0dfb7051966 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4295,6 +4295,7 @@ static void hidpp_connect_event(struct work_struct *work)
 	ret = input_register_device(input);
 	if (ret) {
 		input_free_device(input);
+		hidpp->input = NULL;
 		return;
 	}

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[sashiko-bot]

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

Re: [PATCH] HID: hidpp: fix potential UAF in hidpp_connect_event()

[Jiri Kosina]

On Fri, 12 Jun 2026, sashiko-bot@kernel.org wrote:

> Because hidpp_connect_event() runs asynchronously in a workqueue, incoming
> HID reports can be processed concurrently by hidpp_raw_event() in
> interrupt context. 

This is good point. I will send a v2.

-- 
Jiri Kosina
SUSE Labs