--- /dev/null
+From 3145a3b7ef56fd6d99db804d56afa945e24d7ede Mon Sep 17 00:00:00 2001
+From: Johan Hovold <johan@kernel.org>
+Date: Thu, 25 Apr 2019 18:05:36 +0200
+Subject: USB: serial: fix unthrottle races
+
+[ Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab ]
+
+Fix two long-standing bugs which could potentially lead to memory
+corruption or leave the port throttled until it is reopened (on weakly
+ordered systems), respectively, when read-URB completion races with
+unthrottle().
+
+First, the URB must not be marked as free before processing is complete
+to prevent it from being submitted by unthrottle() on another CPU.
+
+ CPU 1 CPU 2
+ ================ ================
+ complete() unthrottle()
+ process_urb();
+ smp_mb__before_atomic();
+ set_bit(i, free); if (test_and_clear_bit(i, free))
+ submit_urb();
+
+Second, the URB must be marked as free before checking the throttled
+flag to prevent unthrottle() on another CPU from failing to observe that
+the URB needs to be submitted if complete() sees that the throttled flag
+is set.
+
+ CPU 1 CPU 2
+ ================ ================
+ complete() unthrottle()
+ set_bit(i, free); throttled = 0;
+ smp_mb__after_atomic(); smp_mb();
+ if (throttled) if (test_and_clear_bit(i, free))
+ return; submit_urb();
+
+Note that test_and_clear_bit() only implies barriers when the test is
+successful. To handle the case where the URB is still in use an explicit
+barrier needs to be added to unthrottle() for the second race condition.
+
+Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
+Signed-off-by: Johan Hovold <johan@kernel.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/usb/serial/generic.c | 39 +++++++++++++++++++++++++++++-------
+ 1 file changed, 32 insertions(+), 7 deletions(-)
+
+diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
+index 101051dce60c7..faead4f32b1ca 100644
+--- a/drivers/usb/serial/generic.c
++++ b/drivers/usb/serial/generic.c
+@@ -350,6 +350,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
+ struct usb_serial_port *port = urb->context;
+ unsigned char *data = urb->transfer_buffer;
+ unsigned long flags;
++ bool stopped = false;
+ int status = urb->status;
+ int i;
+
+@@ -357,33 +358,51 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
+ if (urb == port->read_urbs[i])
+ break;
+ }
+- set_bit(i, &port->read_urbs_free);
+
+ dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i,
+ urb->actual_length);
+ switch (status) {
+ case 0:
++ usb_serial_debug_data(&port->dev, __func__, urb->actual_length,
++ data);
++ port->serial->type->process_read_urb(urb);
+ break;
+ case -ENOENT:
+ case -ECONNRESET:
+ case -ESHUTDOWN:
+ dev_dbg(&port->dev, "%s - urb stopped: %d\n",
+ __func__, status);
+- return;
++ stopped = true;
++ break;
+ case -EPIPE:
+ dev_err(&port->dev, "%s - urb stopped: %d\n",
+ __func__, status);
+- return;
++ stopped = true;
++ break;
+ default:
+ dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
+ __func__, status);
+- goto resubmit;
++ break;
+ }
+
+- usb_serial_debug_data(&port->dev, __func__, urb->actual_length, data);
+- port->serial->type->process_read_urb(urb);
++ /*
++ * Make sure URB processing is done before marking as free to avoid
++ * racing with unthrottle() on another CPU. Matches the barriers
++ * implied by the test_and_clear_bit() in
++ * usb_serial_generic_submit_read_urb().
++ */
++ smp_mb__before_atomic();
++ set_bit(i, &port->read_urbs_free);
++ /*
++ * Make sure URB is marked as free before checking the throttled flag
++ * to avoid racing with unthrottle() on another CPU. Matches the
++ * smp_mb() in unthrottle().
++ */
++ smp_mb__after_atomic();
++
++ if (stopped)
++ return;
+
+-resubmit:
+ /* Throttle the device if requested by tty */
+ spin_lock_irqsave(&port->lock, flags);
+ port->throttled = port->throttle_req;
+@@ -458,6 +477,12 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
+ port->throttled = port->throttle_req = 0;
+ spin_unlock_irq(&port->lock);
+
++ /*
++ * Matches the smp_mb__after_atomic() in
++ * usb_serial_generic_read_bulk_callback().
++ */
++ smp_mb();
++
+ if (was_throttled)
+ usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+ }
+--
+2.20.1
+
--- /dev/null
+From 6f4e39ecaad2fc7777243da5b4c95dfd2e2dea66 Mon Sep 17 00:00:00 2001
+From: Oliver Neukum <oneukum@suse.com>
+Date: Thu, 14 Jul 2016 15:01:40 +0200
+Subject: USB: serial: use variable for status
+
+[ Upstream commit 3161da970d38cd6ed2ba8cadec93874d1d06e11e ]
+
+This patch turns status in a variable read once from the URB.
+The long term plan is to deliver status to the callback.
+In addition it makes the code a bit more elegant.
+
+Signed-off-by: Oliver Neukum <oneukum@suse.com>
+Signed-off-by: Johan Hovold <johan@kernel.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/usb/serial/generic.c | 18 ++++++++++--------
+ 1 file changed, 10 insertions(+), 8 deletions(-)
+
+diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
+index 54e170dd3dad0..101051dce60c7 100644
+--- a/drivers/usb/serial/generic.c
++++ b/drivers/usb/serial/generic.c
+@@ -350,6 +350,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
+ struct usb_serial_port *port = urb->context;
+ unsigned char *data = urb->transfer_buffer;
+ unsigned long flags;
++ int status = urb->status;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
+@@ -360,22 +361,22 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
+
+ dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i,
+ urb->actual_length);
+- switch (urb->status) {
++ switch (status) {
+ case 0:
+ break;
+ case -ENOENT:
+ case -ECONNRESET:
+ case -ESHUTDOWN:
+ dev_dbg(&port->dev, "%s - urb stopped: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ return;
+ case -EPIPE:
+ dev_err(&port->dev, "%s - urb stopped: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ return;
+ default:
+ dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ goto resubmit;
+ }
+
+@@ -399,6 +400,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
+ {
+ unsigned long flags;
+ struct usb_serial_port *port = urb->context;
++ int status = urb->status;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) {
+@@ -410,22 +412,22 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
+ set_bit(i, &port->write_urbs_free);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+- switch (urb->status) {
++ switch (status) {
+ case 0:
+ break;
+ case -ENOENT:
+ case -ECONNRESET:
+ case -ESHUTDOWN:
+ dev_dbg(&port->dev, "%s - urb stopped: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ return;
+ case -EPIPE:
+ dev_err_console(port, "%s - urb stopped: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ return;
+ default:
+ dev_err_console(port, "%s - nonzero urb status: %d\n",
+- __func__, urb->status);
++ __func__, status);
+ goto resubmit;
+ }
+
+--
+2.20.1
+
projects / thirdparty / kernel / stable-queue.git / commitdiff
