Re: [PATCH 01/11] Input: ims-pcu - release data interface on disconnect

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect size parameter used when freeing the IN endpoint DMA buffer.
- [High] Use-After-Free race condition in sysfs handlers due to incorrect teardown order of the data interface.
--

commit 53bdd94824ab81874a46c9320ecc2816e98b47a9
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: ims-pcu - release data interface on disconnect
    
During probe the driver claims the data interface, but it never releases
it. Release it in disconnect to avoid leaving it permanently claimed.

> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 4c022a36dbe84..89737ef381c1c 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -2090,6 +2090,7 @@ static void ims_pcu_disconnect(struct usb_interface *intf)
>  		ims_pcu_destroy_application_mode(pcu);
>  
>  	ims_pcu_buffers_free(pcu);

[Severity: High]
This is a pre-existing issue, but are we using the correct size parameter
when freeing the IN endpoint DMA buffer?

Looking at ims_pcu_buffers_free():

drivers/input/misc/ims-pcu.c:ims_pcu_buffers_free() {
	...
	usb_free_coherent(pcu->udev, pcu->max_out_size,
			  pcu->urb_in_buf, pcu->read_dma);
	...
}

It appears to free pcu->urb_in_buf using pcu->max_out_size instead of
pcu->max_in_size.

If a malicious USB device presents endpoints with different max packet sizes,
could passing a mismatched size to the DMA freeing function cause memory
corruption or leaks?

> +	usb_driver_release_interface(&ims_pcu_driver, pcu->data_intf);

[Severity: High]
This is a pre-existing issue, but could releasing the data interface after
freeing the buffers create a use-after-free race condition?

During probe, the driver claims the data interface, which automatically
creates sysfs attributes on it. If a concurrent sysfs operation accesses a
file on the data interface during disconnect, it might retrieve the
still-valid pcu pointer and attempt to use pcu->urb_out_buf to send a command.

Since ims_pcu_buffers_free() has already freed the buffers, could this result
in a use-after-free? Would it be safer to release the interface before
freeing the buffers?

>  	kfree(pcu);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523050634.501509-1-dmitry.torokhov@gmail.com?part=1