lkml.org 
[lkml]   [2024]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [Linux Kernel Bug][usb/f_printer] WARNING in usb_ep_queue
From
On 02.02.24 19:59, Chenyuan Yang wrote:
> Dear Linux Developers for F_printer,
>
> We encountered "WARNING in usb_ep_queue" when testing the f_printer driver with
> Syzkaller and our generated specifications.
>

Hi,

it is clear what happens, but at least to me it is not clear why we allow
a write() before we enable the endpoint. Anyway, does this fix the issue?

Regards
Oliver
From 0008697b8ce373a0378058b60d1f1498c3821330 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 13 Mar 2024 16:15:56 +0100
Subject: [PATCH] usb: f_printer: sanity check in write

User space can trigger a write() before the endpoint needed
for that is enabled. Check for that.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/usb/gadget/function/f_printer.c | 37 ++++++++++++++++---------
1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 076dd4c1be96..1e266ba697e8 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -577,6 +577,7 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
size_t bytes_copied = 0;
struct usb_request *req;
int value;
+ int err = -ENODEV;

DBG(dev, "printer_write trying to send %d bytes\n", (int)len);

@@ -586,11 +587,8 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
mutex_lock(&dev->lock_printer_io);
spin_lock_irqsave(&dev->lock, flags);

- if (dev->interface < 0) {
- spin_unlock_irqrestore(&dev->lock, flags);
- mutex_unlock(&dev->lock_printer_io);
- return -ENODEV;
- }
+ if (dev->interface < 0)
+ goto error_spin;

/* Check if a printer reset happens while we have interrupts on */
dev->reset_printer = 0;
@@ -605,8 +603,8 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
* a NON-Blocking call or not.
*/
if (fd->f_flags & (O_NONBLOCK|O_NDELAY)) {
- mutex_unlock(&dev->lock_printer_io);
- return -EAGAIN;
+ err = -EAGAIN;
+ goto error_mutex;
}

/* Sleep until a write buffer is available */
@@ -657,9 +655,17 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
/* We've disconnected or reset so free the req and buffer */
if (dev->reset_printer) {
list_add(&req->list, &dev->tx_reqs);
- spin_unlock_irqrestore(&dev->lock, flags);
- mutex_unlock(&dev->lock_printer_io);
- return -EAGAIN;
+ err = -EAGAIN;
+ goto error_spin;
+ }
+
+ /*
+ * We cannot guarantee user space is using the API nicely
+ * This check needs to be duplicated
+ */
+ if (!dev->in_ep->enabled && dev->in_ep->address) {
+ err = -ESHUTDOWN;
+ goto error_spin;
}

list_add(&req->list, &dev->tx_reqs_active);
@@ -670,9 +676,8 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
spin_lock(&dev->lock);
if (value) {
list_move(&req->list, &dev->tx_reqs);
- spin_unlock_irqrestore(&dev->lock, flags);
- mutex_unlock(&dev->lock_printer_io);
- return -EAGAIN;
+ err = -EAGAIN;
+ goto error_spin;
}
}

@@ -685,6 +690,12 @@ printer_write(struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
return bytes_copied;
else
return -EAGAIN;
+
+error_spin:
+ spin_unlock_irqrestore(&dev->lock, flags);
+error_mutex:
+ mutex_unlock(&dev->lock_printer_io);
+ return err;
}

static int
--
2.44.0
\
 
 \ /
  Last update: 2024-03-13 16:17    [W:0.038 / U:0.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site