[PATCH] HID: core: Avoid leaking field ordering on repeated connect

[PATCH] HID: core: Avoid leaking field ordering on repeated connect

[Nazarii Tupitsa]

hid_connect() builds report->field_entries for every input report. Some
drivers stop and restart the same parsed HID device. Wacom wireless does
this when the connected tablet changes. Each subsequent hid_connect()
overwrites field_entries and leaks the previous allocation.

No physical Wacom device was available, so the reproducer was built with a
Raspberry Pi Pico. It emulates the affected wireless receiver lifecycle and
repeats the tablet connect/disconnect sequence.

The ordering is owned by the parsed struct hid_report and remains valid
until hid_free_report() destroys the report. Skip initialization when the
ordering has already been built. If allocation fails, field_entries remains
NULL so a later connect can retry.

On Linux 7.2.3, the Pico emulated a Wacom 056a:0084 wireless receiver. An
unpatched run with repeated tablet connect/disconnect cycles left multiple
unreferenced 512-byte allocations in a kmemleak scan. With this change,
repeated cycles successfully recreated the Pen, Pad and Finger devices, and
scans after removing the receiver found no leaks.

A representative kmemleak entry was:

  unreferenced object (size 512):
    backtrace:
      __kmalloc_noprof
      hid_connect
      hid_hw_start
      wacom_parse_and_register [wacom]
      wacom_wireless_work [wacom]

Fixes: 22f4b026c3dd ("HID: compute an ordered list of input fields to process")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Nazarii Tupitsa <nazarii.tupitsa@gmail.com>
---
 drivers/hid/hid-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a3ff0514f..9f4ba5f60 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1802,6 +1802,9 @@ static void hid_report_process_ordering(struct hid_device *hid,
 	unsigned int a, u, usages;
 	unsigned int count = 0;
 
+	if (report->field_entries)
+		return;
+
 	/* count the number of individual fields in the report */
 	for (a = 0; a < report->maxfield; a++) {
 		field = report->field[a];

base-commit: 23da087d9a636d9b2046f3aa3b7470ae51b89c8a
-- 
2.50.1

Re: [PATCH] HID: core: Avoid leaking field ordering on repeated connect

[sashiko-bot]

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

Pre-existing issues:
- [High] Cross-interface Use-After-Free / NULL pointer dereference in wacom_wireless_work().
- [Medium] Memory leak of bpf.device_data in hid_connect() error path.
--

commit adb3570e35e55b2d9bb649e1af25cf6ef4c95c01
Author: Nazarii Tupitsa <nazarii.tupitsa@gmail.com>

HID: core: Avoid leaking field ordering on repeated connect

hid_connect() builds report->field_entries for every input report. Some
drivers stop and restart the same parsed HID device. Wacom wireless does
this when the connected tablet changes. Each subsequent hid_connect()
overwrites field_entries and leaks the previous allocation.

[ ... ]

>     A representative kmemleak entry was:
>     
>       unreferenced object (size 512):
>         backtrace:
>           __kmalloc_noprof
>           hid_connect
>           hid_hw_start
>           wacom_parse_and_register [wacom]
>           wacom_wireless_work [wacom]

[Severity: High]
This is a pre-existing issue, but can wacom_wireless_work() cause a
use-after-free or NULL pointer dereference when accessing sibling interfaces?

It accesses sibling USB interfaces directly without holding any locks or taking
reference counts:

drivers/hid/wacom_sys.c:wacom_wireless_work() {
    ...
	/* Stylus interface */
	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
	wacom1 = hid_get_drvdata(hdev1);
	wacom_wac1 = &(wacom1->wacom_wac);
	wacom_release_resources(wacom1);
    ...
}

If the sibling interface is concurrently unbound (e.g. by userspace unbinding
the driver from interface 1 via sysfs, or during a race in hot-unplug),
usbhid_disconnect() destroys the hid_device and clears the intfdata.

Would wacom_wireless_work() then read a NULL pointer (causing a crash in
hid_get_drvdata()) or dereference a freed hid_device or wacom structure?

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a3ff0514f9cdf..9f4ba5f60adca 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c

[Severity: Medium]
This isn't a bug introduced by this patch, but does the error path in
hid_connect() leak the dynamically allocated hdev->bpf.device_data?

In hid_connect(), hid_bpf_connect_device() is called first, which successfully
allocates hdev->bpf.device_data.

If the device has no listeners, hid_connect() returns -ENODEV without calling
hid_bpf_disconnect_device():

drivers/hid/hid-core.c:hid_connect() {
    ...
	if (!hdev->claimed && !hdev->driver->raw_event) {
		hid_err(hdev, "device has no listeners, quitting\n");
		return -ENODEV;
	}
    ...
}

The caller hid_hw_start() stops the LL driver and propagates the error,
preventing the execution of hid_hw_stop() (which normally triggers
hid_disconnect() and frees the BPF data).

Ultimately, when the device is unbound, hid_destroy_device() calls
hid_bpf_destroy_device(), which does not free bpf.device_data.

Could this leave the bpf.device_data memory permanently leaked on device
probe failures for unclaimed devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908200655.139098-1-nazarii.tupitsa@gmail.com?part=1