Re: [PATCH 10/22] usb: gadget: fix a few spelling mistakes in comments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 2 related pre-existing issues it does not
introduce:
- [Critical] usb: gadget: f_hid: concurrent DMA buffer modification in
  f_hidg_get_report() [1]
- [High] usb: gadget: f_printer: hang in printer_read() on soft reset or
  disconnect [2]
--

commit 8f8dba8a29efc1fdd5e1c811223c191a86a07a95
Author: Shunbo Shi <shishunbo@cmss.chinamobile.com>

usb: gadget: fix a few spelling mistakes in comments

This commit fixes spelling mistakes in comments across the USB gadget
drivers.

[ ... ]

> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
> index 3c6b43d06a6d1..cf54704b7655b 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -618,7 +618,7 @@ static void get_report_workqueue_handler(struct work_struct *work)
>  			 */
>  			req->buf = ptr->report_data.data;
>  		} else {
> -			/* If there are no prevoiusly sent reports send empty report */
> +			/* If there are no previously sent reports send empty report */
>  			req->buf = hidg->get_report.data;
>  			memset(req->buf, 0x0, req->length);
>  		}

[Severity: Critical]
This problem wasn't introduced by this patch, but can f_hidg_get_report()
overwrite ptr->report_data.data while it is queued for DMA [1]?

CPU0                              CPU1
get_report_workqueue_handler()
  spin_lock_irqsave(&hidg->get_report_spinlock, ...)
  req->buf = ptr->report_data.data;
  usb_ep_queue(cdev->gadget->ep0, req, ...)
  spin_unlock_irqrestore(&hidg->get_report_spinlock, ...)
                                  f_hidg_get_report()
                                    spin_lock_irqsave(&hidg->get_report_spinlock, ...)
                                    ptr = f_hidg_search_for_report(...)
                                    copy_from_user(&ptr->report_data, ...)
                                      <-- sleeps in atomic & overwrites DMA buffer

When a host GET_REPORT request races with a GADGET_HID_WRITE_GET_REPORT
ioctl, f_hidg_get_report() calls copy_from_user() into ptr->report_data
while the buffer is still in flight on ep0, corrupting the transmitted data
and violating DMA coherency.  Calling copy_from_user() under
hidg->get_report_spinlock also sleeps in atomic context.

[ ... ]

> diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
> index 1857d786110b4..3f25d99be34d8 100644
> --- a/drivers/usb/gadget/function/f_printer.c
> +++ b/drivers/usb/gadget/function/f_printer.c
> @@ -557,7 +557,7 @@ printer_read(struct file *fd, char __user *buf, size_t len, loff_t *ptr)
>  
>  		/* If we not returning all the data left in this RX request
>  		 * buffer then adjust the amount of data left in the buffer.
> -		 * Othewise if we are done with this RX request buffer then
> +		 * Otherwise if we are done with this RX request buffer then
>  		 * requeue it to get any incoming data from the USB host.
>  		 */
>  		if (size < current_rx_bytes) {

[Severity: High]
This problem wasn't introduced by this patch, but can printer_read() hang
forever on dev->rx_wait when a soft reset or disconnect occurs [2]?

drivers/usb/gadget/function/f_printer.c:printer_read() {
	...
	if ((current_rx_bytes == 0) &&
			(likely(list_empty(&dev->rx_buffers)))) {
		...
		wait_event_interruptible(dev->rx_wait,
				(likely(!list_empty(&dev->rx_buffers))));
		spin_lock_irqsave(&dev->lock, flags);
		if (dev->interface < 0)
			goto out_disabled;
	}
	...
}

printer_soft_reset() and printer_func_disable() move all entries from
dev->rx_buffers to dev->rx_reqs, set dev->reset_printer = 1 or
dev->interface = -1, and wake dev->rx_wait.  Because the wait condition
only checks !list_empty(&dev->rx_buffers), printer_read() goes back to
sleep while holding dev->lock_printer_io, deadlocking subsequent I/O on the
gadget.

[1] https://sashiko.dev/#/bug/linux-59b5d9cb-e0c6-4190-8add-226d731d01e5
[2] https://sashiko.dev/#/bug/linux-05ce2087-0db6-45a4-8d55-98d851217360

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110064105.20335-1-shishunbo@cmss.chinamobile.com?part=10