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

[sashiko-bot@kernel.org]

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