Re: [PATCH] Input: cm109 - fix URB double-submit due to missing open check

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] TOCTOU Race leading to Use-After-Free during device disconnect
- [High] State Machine Stall due to unhandled URB submission failure
- [High] DMA API Violation from passing coherent buffer to streaming DMA mapping
--

--- Patch [1]: [PATCH] Input: cm109 - fix URB double-submit due to missing open check ---
commit 8242b0347566c678c3b0f2e9b93934d136b010bb
Author: Xue Lei <Xue.Lei@windriver.com>

Input: cm109 - fix URB double-submit due to missing open check

This commit adds a dev->open check in cm109_input_ev() to reject input
events when the device is not fully initialized, preventing a double-submit
of urb_ctl.

Closes: https://syzkaller.appspot.com/bug?extid=e69c25cf38a53d0cf64c

> diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
> index 353d3c1d347d5..558cb73d5c6df 100644
> --- a/drivers/input/misc/cm109.c
> +++ b/drivers/input/misc/cm109.c
> @@ -601,6 +601,9 @@ static int cm109_input_ev(struct input_dev *idev, unsigned int type,
>  {
>  	struct cm109_dev *dev = input_get_drvdata(idev);
>  
> +	if (!dev->open)
> +		return -ENODEV;
> +

[Severity: High]
This is a pre-existing issue, but does this lockless check introduce a TOCTOU
race during device disconnect?

Looking at cm109_input_close(), dev->open is cleared after traffic is
stopped:

cm109_input_close()
    cm109_stop_traffic(dev);
    dev->open = 0;

If a concurrent event injection happens simultaneously, couldn't
cm109_input_ev() see dev->open == 1 just before cm109_input_close() sets
it to 0?

Since cm109_usb_cleanup() frees the dev structure without explicitly calling
usb_kill_urb(), could this allow an asynchronously submitted URB to complete
and access the freed dev structure?

>  	dev_dbg(&dev->intf->dev,
>  		"input_ev: type=%u code=%u value=%d\n", type, code, value);
>  

[Severity: High]
This is a pre-existing issue, but can the URB submission loop stall
permanently on failure?

Looking at cm109_submit_buzz_toggle(), if usb_submit_urb() fails due to
memory pressure or bus errors, it logs the error but does not clear
dev->ctl_urb_pending:

cm109_submit_buzz_toggle()
    error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
    if (error)
        dev_err(&dev->intf->dev, ...);

If ctl_urb_pending remains 1, wouldn't subsequent calls to
cm109_toggle_buzzer_async() always take this path:

cm109_toggle_buzzer_async()
    if (dev->ctl_urb_pending) {
        dev->buzzer_pending = 1;
        return;
    }

Since the URB was never actually submitted, its completion callback won't
run to submit urb_irq, meaning the device would permanently stop receiving
input events and buzzer updates until re-plugged?

[Severity: High]
This is a pre-existing issue, but does the synchronous buzzer toggle violate
DMA mapping rules?

In cm109_usb_probe(), dev->ctl_data is allocated using usb_alloc_coherent().
Later, cm109_toggle_buzzer_sync() passes this coherent buffer directly to
usb_control_msg():

cm109_toggle_buzzer_sync()
    error = usb_control_msg(dev->udev,
                            usb_sndctrlpipe(dev->udev, 0),
                            ...
                            dev->ctl_data,
                            USB_PKT_LEN, USB_CTRL_SET_TIMEOUT);

Since usb_control_msg() uses streaming DMA mapping via dma_map_single()
without the URB_NO_TRANSFER_DMA_MAP flag, could passing a buffer allocated
with a coherent API trigger mapping errors on architectures with
CONFIG_DMA_API_DEBUG enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709073133.373657-1-Xue.Lei@windriver.com?part=1