Re: [PATCH 02/11] Input: ims-pcu - fix use-after-free and double-free in disconnect
From: sashiko-bot@kernel.org
Date: 2026-05-23 05:45:38
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] Concurrent unbinding of both the control and data interfaces via sysfs triggers a Use-After-Free on the `pcu` structure. Pre-existing issues: - [Critical] The driver blindly trusts the CDC Union Descriptor to identify interfaces, leading to memory corruption during probe and an unconditional NULL pointer dereference in disconnect. - [High] In `ims_pcu_buffers_free`, the input DMA buffer is freed using the maximum output buffer size instead of its actual allocated size. -- commit 73572626cc6ab15fc9107238c7a713e379862dd8 Author: Dmitry Torokhov [off-list ref] Input: ims-pcu - fix use-after-free and double-free in disconnect This commit changes the interface verification in ims_pcu_disconnect() to directly compare the disconnecting interface against the saved control interface pointer. This prevents premature cleanup caused by malicious devices presenting multiple interfaces with the same class.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c index 89737ef381c1c..2ad72fe4a51d9 100644 --- a/drivers/input/misc/ims-pcu.c +++ b/drivers/input/misc/ims-pcu.c@@ -2071,7 +2071,6 @@ static int ims_pcu_probe(struct usb_interface *intf, static void ims_pcu_disconnect(struct usb_interface *intf) { struct ims_pcu *pcu = usb_get_intfdata(intf); - struct usb_host_interface *alt = intf->cur_altsetting; usb_set_intfdata(intf, NULL);@@ -2079,7 +2078,7 @@ static void ims_pcu_disconnect(struct usb_interface *intf) * See if we are dealing with control or data interface. The cleanup * happens when we unbind primary (control) interface. */ - if (alt->desc.bInterfaceClass != USB_CLASS_COMM) + if (intf != pcu->ctrl_intf)
[Severity: High] Can this introduce a use-after-free if both control and data interfaces are unbound concurrently via sysfs? If Process A unbinds the data interface, it gets the pcu pointer from usb_get_intfdata() and might be preempted. The USB core sets the data interface condition to USB_INTERFACE_UNBINDING. Process B then unbinds the control interface, enters ims_pcu_disconnect(), and calls usb_driver_release_interface() on the data interface. Since the data interface is unbinding, usb_driver_release_interface() returns immediately without waiting, and Process B proceeds to kfree(pcu). When Process A resumes, evaluating intf != pcu->ctrl_intf would dereference the freed pcu pointer. [Severity: Critical] This is a pre-existing issue, but does this new check introduce an unconditional NULL pointer dereference if the CDC Union Descriptor is spoofed? During ims_pcu_probe(), the driver uses bMasterInterface0 from the CDC Union Descriptor as pcu->ctrl_intf without verifying it matches the probed interface. If a malicious device supplies a descriptor pointing to an interface owned by another driver, the actual probed interface's intfdata remains NULL. In that scenario, usb_get_intfdata() returns NULL for pcu, and evaluating intf != pcu->ctrl_intf here will panic. Note that this is fixed later in the patch series by commit af29ec151c71a.
return; ims_pcu_stop_io(pcu);
[Severity: High]
This is a pre-existing issue, but could ims_pcu_buffers_free() be freeing
the input buffer with the wrong size?
Looking at ims_pcu_buffers_alloc(), pcu->urb_in_buf is allocated using
pcu->max_in_size. However, ims_pcu_buffers_free() appears to free it using
pcu->max_out_size:
usb_free_coherent(pcu->udev, pcu->max_out_size, pcu->urb_in_buf,
pcu->read_dma);
If a device specifies different maximum packet sizes for IN and OUT endpoints,
calling usb_free_coherent() with the wrong size could corrupt the DMA
allocator's buddy system.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523050634.501509-1-dmitry.torokhov@gmail.com?part=2