[PATCH] usb: gadget: f_printer: Fix fsync waiting for active transfers

[PATCH] usb: gadget: f_printer: Fix fsync waiting for active transfers

[raoxu]

From: Xu Rao <raoxu@uniontech.com>

printer_fsync() checks the list of free TX requests before deciding
whether to wait for all data to be sent. Once a write queues a request,
it is removed from the free list and placed on tx_reqs_active, so the
free list can be empty while a USB transfer is still in progress.

This allows fsync() to return before the queued data has completed.
Check tx_reqs_active instead, matching the wait condition and the
completion callback that wakes tx_flush_wait.

Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
 drivers/usb/gadget/function/f_printer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 1857d786110b..a2437c8e668d 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -736,7 +736,7 @@ printer_fsync(struct file *fd, loff_t start, loff_t end, int datasync)
 		return -ENODEV;
 	}

-	tx_list_empty = (likely(list_empty(&dev->tx_reqs)));
+	tx_list_empty = (likely(list_empty(&dev->tx_reqs_active)));
 	spin_unlock_irqrestore(&dev->lock, flags);

 	if (!tx_list_empty) {
--
2.50.1

Re: [PATCH] usb: gadget: f_printer: Fix fsync waiting for active transfers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] usb: gadget: f_printer: missing lock on error path in
  printer_write() [1]
--

commit 0f81a8331b5725aa0fef1a2421c8f88ca088bb9f
Author: Xu Rao <raoxu@uniontech.com>

usb: gadget: f_printer: Fix fsync waiting for active transfers

This commit updates printer_fsync() to check dev->tx_reqs_active instead
of dev->tx_reqs before waiting for pending TX transfers to complete.

> diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
> index 1857d786110b4..a2437c8e668da 100644
> --- a/drivers/usb/gadget/function/f_printer.c
> +++ b/drivers/usb/gadget/function/f_printer.c
> @@ -736,7 +736,7 @@ printer_fsync(struct file *fd, loff_t start, loff_t end, int datasync)
>  		return -ENODEV;
>  	}
>  
> -	tx_list_empty = (likely(list_empty(&dev->tx_reqs)));
> +	tx_list_empty = (likely(list_empty(&dev->tx_reqs_active)));
>  	spin_unlock_irqrestore(&dev->lock, flags);
>  
>  	if (!tx_list_empty) {

[Severity: Critical]
This problem wasn't introduced by this patch, but can printer_write()
corrupt dev->tx_reqs [1] when copy_from_user() fails?

drivers/usb/gadget/function/f_printer.c:printer_write() {
	...
		/* Don't leave irqs off while doing memory copies */
		spin_unlock_irqrestore(&dev->lock, flags);

		if (copy_from_user(req->buf, buf, size)) {
			list_add(&req->list, &dev->tx_reqs);
			mutex_unlock(&dev->lock_printer_io);
			return bytes_copied;
		}
	...
}

printer_write() drops dev->lock before copy_from_user() and adds req back
to dev->tx_reqs on the error path without re-acquiring dev->lock, racing
with TX completion handlers that also modify dev->tx_reqs.

[1] https://sashiko.dev/#/bug/linux-76584f8a-a4f9-490d-8c0c-3cdb1c0cb7cb

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/09D38BF2C904A309+20261110031832.219431-1-raoxu@uniontech.com?part=1