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

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 [off-list ref]

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?

[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