Stable Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 2/2] USB: serial: opticon: stop all I/O on close()
       [not found] <20200114110146.5929-1-johan@kernel.org>
@ 2020-01-14 11:01 ` Johan Hovold
  2020-01-14 12:18   ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Johan Hovold @ 2020-01-14 11:01 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb, stable

Make sure to stop any submitted write URBs on close(). This specifically
avoids a NULL-pointer dereference or use-after-free in case of a late
completion event after driver unbind.

Fixes: 648d4e16567e ("USB: serial: opticon: add write support")
Cc: stable <stable@vger.kernel.org>	# 2.6.30: xxx: USB: serial: opticon: add chars_in_buffer() implementation
Signed-off-by: Johan Hovold <johan@kernel.org>
---

v2
 - add missing address-of operator that was never commit before
   generating the patch


 drivers/usb/serial/opticon.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/usb/serial/opticon.c b/drivers/usb/serial/opticon.c
index f7bccf14a71f..0af76800bd78 100644
--- a/drivers/usb/serial/opticon.c
+++ b/drivers/usb/serial/opticon.c
@@ -42,6 +42,8 @@ struct opticon_private {
 	bool cts;
 	int outstanding_urbs;
 	int outstanding_bytes;
+
+	struct usb_anchor anchor;
 };
 
 
@@ -150,6 +152,15 @@ static int opticon_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return res;
 }
 
+static void opticon_close(struct usb_serial_port *port)
+{
+	struct opticon_private *priv = usb_get_serial_port_data(port);
+
+	usb_kill_anchored_urbs(&priv->anchor);
+
+	usb_serial_generic_close(port);
+}
+
 static void opticon_write_control_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
@@ -226,10 +237,13 @@ static int opticon_write(struct tty_struct *tty, struct usb_serial_port *port,
 		(unsigned char *)dr, buffer, count,
 		opticon_write_control_callback, port);
 
+	usb_anchor_urb(urb, &priv->anchor);
+
 	/* send it down the pipe */
 	ret = usb_submit_urb(urb, GFP_ATOMIC);
 	if (ret) {
 		dev_err(&port->dev, "failed to submit write urb: %d\n", ret);
+		usb_unanchor_urb(urb);
 		goto error;
 	}
 
@@ -364,6 +378,7 @@ static int opticon_port_probe(struct usb_serial_port *port)
 		return -ENOMEM;
 
 	spin_lock_init(&priv->lock);
+	init_usb_anchor(&priv->anchor);
 
 	usb_set_serial_port_data(port, priv);
 
@@ -391,6 +406,7 @@ static struct usb_serial_driver opticon_device = {
 	.port_probe =		opticon_port_probe,
 	.port_remove =		opticon_port_remove,
 	.open =			opticon_open,
+	.close =		opticon_close,
 	.write =		opticon_write,
 	.write_room = 		opticon_write_room,
 	.chars_in_buffer =	opticon_chars_in_buffer,
-- 
2.24.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 2/2] USB: serial: opticon: stop all I/O on close()
  2020-01-14 11:01 ` [PATCH v2 2/2] USB: serial: opticon: stop all I/O on close() Johan Hovold
@ 2020-01-14 12:18   ` Greg KH
  2020-01-16 16:21     ` Johan Hovold
  0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2020-01-14 12:18 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb, stable

On Tue, Jan 14, 2020 at 12:01:46PM +0100, Johan Hovold wrote:
> Make sure to stop any submitted write URBs on close(). This specifically
> avoids a NULL-pointer dereference or use-after-free in case of a late
> completion event after driver unbind.
> 
> Fixes: 648d4e16567e ("USB: serial: opticon: add write support")
> Cc: stable <stable@vger.kernel.org>	# 2.6.30: xxx: USB: serial: opticon: add chars_in_buffer() implementation
> Signed-off-by: Johan Hovold <johan@kernel.org>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 2/2] USB: serial: opticon: stop all I/O on close()
  2020-01-14 12:18   ` Greg KH
@ 2020-01-16 16:21     ` Johan Hovold
  0 siblings, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2020-01-16 16:21 UTC (permalink / raw)
  To: Greg KH; +Cc: Johan Hovold, linux-usb, stable

On Tue, Jan 14, 2020 at 01:18:57PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 14, 2020 at 12:01:46PM +0100, Johan Hovold wrote:
> > Make sure to stop any submitted write URBs on close(). This specifically
> > avoids a NULL-pointer dereference or use-after-free in case of a late
> > completion event after driver unbind.
> > 
> > Fixes: 648d4e16567e ("USB: serial: opticon: add write support")
> > Cc: stable <stable@vger.kernel.org>	# 2.6.30: xxx: USB: serial: opticon: add chars_in_buffer() implementation
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks for the review.

I just submitted a patch preventing individual ports from being unbound,
which almost no USB-serial driver can handle generally without crashing.

And as USB core handles the case were the USB interface driver is
unbound, this one doesn't fix anything critical.

So I'll apply this for -next with the following updated commit message:

    USB: serial: opticon: stop all I/O on close()
    
    Make sure to stop any submitted write URBs on close().
    
    Note that the tty layer will wait up to 30 seconds for the buffers to
    drain before close() is called.

and drop the Fixes and stable tags instead.

Johan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200114110146.5929-1-johan@kernel.org>
2020-01-14 11:01 ` [PATCH v2 2/2] USB: serial: opticon: stop all I/O on close() Johan Hovold
2020-01-14 12:18   ` Greg KH
2020-01-16 16:21     ` Johan Hovold

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git