[PATCH 0/8] USB: serial: fix interface URB leaks and use-after-free

[PATCH 0/8] USB: serial: fix interface URB leaks and use-after-free

[Johan Hovold]

This series fixes a number of issues where resources were not properly
released on probe errors. Typically, URBs allocated and submitted in an
attach callback were never unlinked or released in a matching release
callback. This could lead to memory leaks and use-after-free bugs as
we could end up with unbound interfaces with active URBs.

Included is also a couple of minor fixes and clean ups of the keyspan
driver, and a fix of how we deal with minor-number exhaustion in core.

Johan


Johan Hovold (8):
  USB: serial: io_edgeport: fix memory leaks in attach error path
  USB: serial: io_edgeport: fix memory leaks in probe error path
  USB: serial: keyspan: fix use-after-free in probe error path
  USB: serial: keyspan: fix URB unlink
  USB: serial: keyspan: fix debug and error messages
  USB: serial: mxuport: fix use-after-free in probe error path
  USB: serial: quatech2: fix use-after-free in probe error path
  USB: serial: fix minor-number allocation

 drivers/usb/serial/io_edgeport.c | 56 +++++++++++++++++++++----------
 drivers/usb/serial/keyspan.c     | 72 ++++++++++++++++++++++------------------
 drivers/usb/serial/mxuport.c     | 10 ++++++
 drivers/usb/serial/quatech2.c    |  1 +
 drivers/usb/serial/usb-serial.c  |  3 +-
 5 files changed, 90 insertions(+), 52 deletions(-)

-- 
2.7.3

[PATCH 1/8] USB: serial: io_edgeport: fix memory leaks in attach error path

[Johan Hovold]

Private data, URBs and buffers allocated for Epic devices during
attach were never released on errors (e.g. missing endpoints).

Fixes: 6e8cf7751f9f ("USB: add EPIC support to the io_edgeport driver")
Cc: stable <stable@vger.kernel.org>	# v2.6.21
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_edgeport.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index f3007ecdd1b4..edd568bc0de5 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2849,14 +2849,16 @@ static int edge_startup(struct usb_serial *serial)
 				/* not set up yet, so do it now */
 				edge_serial->interrupt_read_urb =
 						usb_alloc_urb(0, GFP_KERNEL);
-				if (!edge_serial->interrupt_read_urb)
-					return -ENOMEM;
+				if (!edge_serial->interrupt_read_urb) {
+					response = -ENOMEM;
+					break;
+				}
 
 				edge_serial->interrupt_in_buffer =
 					kmalloc(buffer_size, GFP_KERNEL);
 				if (!edge_serial->interrupt_in_buffer) {
-					usb_free_urb(edge_serial->interrupt_read_urb);
-					return -ENOMEM;
+					response = -ENOMEM;
+					break;
 				}
 				edge_serial->interrupt_in_endpoint =
 						endpoint->bEndpointAddress;
@@ -2884,14 +2886,16 @@ static int edge_startup(struct usb_serial *serial)
 				/* not set up yet, so do it now */
 				edge_serial->read_urb =
 						usb_alloc_urb(0, GFP_KERNEL);
-				if (!edge_serial->read_urb)
-					return -ENOMEM;
+				if (!edge_serial->read_urb) {
+					response = -ENOMEM;
+					break;
+				}
 
 				edge_serial->bulk_in_buffer =
 					kmalloc(buffer_size, GFP_KERNEL);
 				if (!edge_serial->bulk_in_buffer) {
-					usb_free_urb(edge_serial->read_urb);
-					return -ENOMEM;
+					response = -ENOMEM;
+					break;
 				}
 				edge_serial->bulk_in_endpoint =
 						endpoint->bEndpointAddress;
@@ -2917,9 +2921,22 @@ static int edge_startup(struct usb_serial *serial)
 			}
 		}
 
-		if (!interrupt_in_found || !bulk_in_found || !bulk_out_found) {
-			dev_err(ddev, "Error - the proper endpoints were not found!\n");
-			return -ENODEV;
+		if (response || !interrupt_in_found || !bulk_in_found ||
+							!bulk_out_found) {
+			if (!response) {
+				dev_err(ddev, "expected endpoints not found\n");
+				response = -ENODEV;
+			}
+
+			usb_free_urb(edge_serial->interrupt_read_urb);
+			kfree(edge_serial->interrupt_in_buffer);
+
+			usb_free_urb(edge_serial->read_urb);
+			kfree(edge_serial->bulk_in_buffer);
+
+			kfree(edge_serial);
+
+			return response;
 		}
 
 		/* start interrupt read for this edgeport this interrupt will
-- 
2.7.3

[PATCH 2/8] USB: serial: io_edgeport: fix memory leaks in probe error path

[Johan Hovold]

URBs and buffers allocated in attach for Epic devices would never be
deallocated in case of a later probe error (e.g. failure to allocate
minor numbers) as disconnect is then never called.

Fix by moving deallocation to release and making sure that the
URBs are first unlinked.

Fixes: f9c99bb8b3a1 ("USB: usb-serial: replace shutdown with disconnect,
release")
Cc: stable <stable@vger.kernel.org>	# v2.6.31

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_edgeport.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index edd568bc0de5..11c05ce2f35f 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2959,16 +2959,9 @@ static void edge_disconnect(struct usb_serial *serial)
 {
 	struct edgeport_serial *edge_serial = usb_get_serial_data(serial);
 
-	/* stop reads and writes on all ports */
-	/* free up our endpoint stuff */
 	if (edge_serial->is_epic) {
 		usb_kill_urb(edge_serial->interrupt_read_urb);
-		usb_free_urb(edge_serial->interrupt_read_urb);
-		kfree(edge_serial->interrupt_in_buffer);
-
 		usb_kill_urb(edge_serial->read_urb);
-		usb_free_urb(edge_serial->read_urb);
-		kfree(edge_serial->bulk_in_buffer);
 	}
 }
 
@@ -2981,6 +2974,16 @@ static void edge_release(struct usb_serial *serial)
 {
 	struct edgeport_serial *edge_serial = usb_get_serial_data(serial);
 
+	if (edge_serial->is_epic) {
+		usb_kill_urb(edge_serial->interrupt_read_urb);
+		usb_free_urb(edge_serial->interrupt_read_urb);
+		kfree(edge_serial->interrupt_in_buffer);
+
+		usb_kill_urb(edge_serial->read_urb);
+		usb_free_urb(edge_serial->read_urb);
+		kfree(edge_serial->bulk_in_buffer);
+	}
+
 	kfree(edge_serial);
 }
 
-- 
2.7.3

[PATCH 3/8] USB: serial: keyspan: fix use-after-free in probe error path

[Johan Hovold]

The interface instat and indat URBs were submitted in attach, but never
unlinked in release before deallocating the corresponding transfer
buffers.

In the case of a late probe error (e.g. due to failed minor allocation),
disconnect would not have been called before release, causing the
buffers to be freed while the URBs are still in use. We'd also end up
with active URBs for an unbound interface.

Fixes: f9c99bb8b3a1 ("USB: usb-serial: replace shutdown with disconnect,
release")
Cc: stable <stable@vger.kernel.org>	# v2.6.31

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/keyspan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index b6bd8e4a6486..1cf05883f48c 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -2376,6 +2376,10 @@ static void keyspan_release(struct usb_serial *serial)
 
 	s_priv = usb_get_serial_data(serial);
 
+	/* Make sure to unlink the URBs submitted in attach. */
+	usb_kill_urb(s_priv->instat_urb);
+	usb_kill_urb(s_priv->indat_urb);
+
 	usb_free_urb(s_priv->instat_urb);
 	usb_free_urb(s_priv->indat_urb);
 	usb_free_urb(s_priv->glocont_urb);
-- 
2.7.3

[PATCH 4/8] USB: serial: keyspan: fix URB unlink

[Johan Hovold]

A driver must not rely on the URB status field to try to determine if an
URB is active.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/keyspan.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index 1cf05883f48c..86d54932843d 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -1082,12 +1082,6 @@ static int keyspan_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return 0;
 }
 
-static inline void stop_urb(struct urb *urb)
-{
-	if (urb && urb->status == -EINPROGRESS)
-		usb_kill_urb(urb);
-}
-
 static void keyspan_dtr_rts(struct usb_serial_port *port, int on)
 {
 	struct keyspan_port_private *p_priv = usb_get_serial_port_data(port);
@@ -1114,10 +1108,10 @@ static void keyspan_close(struct usb_serial_port *port)
 	p_priv->out_flip = 0;
 	p_priv->in_flip = 0;
 
-	stop_urb(p_priv->inack_urb);
+	usb_kill_urb(p_priv->inack_urb);
 	for (i = 0; i < 2; i++) {
-		stop_urb(p_priv->in_urbs[i]);
-		stop_urb(p_priv->out_urbs[i]);
+		usb_kill_urb(p_priv->in_urbs[i]);
+		usb_kill_urb(p_priv->out_urbs[i]);
 	}
 }
 
@@ -2365,9 +2359,9 @@ static void keyspan_disconnect(struct usb_serial *serial)
 
 	s_priv = usb_get_serial_data(serial);
 
-	stop_urb(s_priv->instat_urb);
-	stop_urb(s_priv->glocont_urb);
-	stop_urb(s_priv->indat_urb);
+	usb_kill_urb(s_priv->instat_urb);
+	usb_kill_urb(s_priv->glocont_urb);
+	usb_kill_urb(s_priv->indat_urb);
 }
 
 static void keyspan_release(struct usb_serial *serial)
@@ -2495,11 +2489,11 @@ static int keyspan_port_remove(struct usb_serial_port *port)
 
 	p_priv = usb_get_serial_port_data(port);
 
-	stop_urb(p_priv->inack_urb);
-	stop_urb(p_priv->outcont_urb);
+	usb_kill_urb(p_priv->inack_urb);
+	usb_kill_urb(p_priv->outcont_urb);
 	for (i = 0; i < 2; i++) {
-		stop_urb(p_priv->in_urbs[i]);
-		stop_urb(p_priv->out_urbs[i]);
+		usb_kill_urb(p_priv->in_urbs[i]);
+		usb_kill_urb(p_priv->out_urbs[i]);
 	}
 
 	usb_free_urb(p_priv->inack_urb);
-- 
2.7.3

[PATCH 5/8] USB: serial: keyspan: fix debug and error messages

[Johan Hovold]

The URB status is signed and should be printed using %d rather than %x.

Also print endpoint addresses consistently using %x rather than %d, and
merge a broken-up error message string.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/keyspan.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index 86d54932843d..1f9414bdd649 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -255,7 +255,7 @@ static int keyspan_write(struct tty_struct *tty,
 			return count;
 		}
 
-		dev_dbg(&port->dev, "%s - endpoint %d flip %d\n",
+		dev_dbg(&port->dev, "%s - endpoint %x flip %d\n",
 			__func__, usb_pipeendpoint(this_urb->pipe), flip);
 
 		if (this_urb->status == -EINPROGRESS) {
@@ -300,7 +300,7 @@ static void	usa26_indat_callback(struct urb *urb)
 	endpoint = usb_pipeendpoint(urb->pipe);
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n",
+		dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n",
 			__func__, status, endpoint);
 		return;
 	}
@@ -393,7 +393,8 @@ static void	usa26_instat_callback(struct urb *urb)
 	serial =  urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 	if (urb->actual_length != 9) {
@@ -452,7 +453,7 @@ static void usa28_indat_callback(struct urb *urb)
 
 	do {
 		if (status) {
-			dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n",
+			dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n",
 				__func__, status, usb_pipeendpoint(urb->pipe));
 			return;
 		}
@@ -511,7 +512,8 @@ static void	usa28_instat_callback(struct urb *urb)
 	serial =  urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 
@@ -591,7 +593,8 @@ static void	usa49_instat_callback(struct urb *urb)
 	serial =  urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 
@@ -646,7 +649,7 @@ static void	usa49_indat_callback(struct urb *urb)
 	endpoint = usb_pipeendpoint(urb->pipe);
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n",
+		dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n",
 			__func__, status, endpoint);
 		return;
 	}
@@ -698,7 +701,8 @@ static void usa49wg_indat_callback(struct urb *urb)
 	serial = urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 
@@ -774,8 +778,8 @@ static void usa90_indat_callback(struct urb *urb)
 	endpoint = usb_pipeendpoint(urb->pipe);
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x on endpoint %d.\n",
-		    __func__, status, endpoint);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status %d on endpoint %x\n",
+			__func__, status, endpoint);
 		return;
 	}
 
@@ -847,7 +851,8 @@ static void	usa90_instat_callback(struct urb *urb)
 	serial =  urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 	if (urb->actual_length < 14) {
@@ -912,7 +917,8 @@ static void	usa67_instat_callback(struct urb *urb)
 	serial = urb->context;
 
 	if (status) {
-		dev_dbg(&urb->dev->dev, "%s - nonzero status: %x\n", __func__, status);
+		dev_dbg(&urb->dev->dev, "%s - nonzero status: %d\n",
+				__func__, status);
 		return;
 	}
 
@@ -1215,8 +1221,8 @@ static struct usb_endpoint_descriptor const *find_ep(struct usb_serial const *se
 		if (ep->bEndpointAddress == endpoint)
 			return ep;
 	}
-	dev_warn(&serial->interface->dev, "found no endpoint descriptor for "
-		 "endpoint %x\n", endpoint);
+	dev_warn(&serial->interface->dev, "found no endpoint descriptor for endpoint %x\n",
+			endpoint);
 	return NULL;
 }
 
@@ -1231,7 +1237,8 @@ static struct urb *keyspan_setup_urb(struct usb_serial *serial, int endpoint,
 	if (endpoint == -1)
 		return NULL;		/* endpoint not needed */
 
-	dev_dbg(&serial->interface->dev, "%s - alloc for endpoint %d.\n", __func__, endpoint);
+	dev_dbg(&serial->interface->dev, "%s - alloc for endpoint %x\n",
+			__func__, endpoint);
 	urb = usb_alloc_urb(0, GFP_KERNEL);		/* No ISO */
 	if (!urb)
 		return NULL;
@@ -1566,7 +1573,8 @@ static int keyspan_usa26_send_setup(struct usb_serial *serial,
 		return -1;
 	}
 
-	dev_dbg(&port->dev, "%s - endpoint %d\n", __func__, usb_pipeendpoint(this_urb->pipe));
+	dev_dbg(&port->dev, "%s - endpoint %x\n",
+			__func__, usb_pipeendpoint(this_urb->pipe));
 
 	/* Save reset port val for resend.
 	   Don't overwrite resend for open/close condition. */
@@ -1832,7 +1840,7 @@ static int keyspan_usa49_send_setup(struct usb_serial *serial,
 		return -1;
 	}
 
-	dev_dbg(&port->dev, "%s - endpoint %d (%d)\n",
+	dev_dbg(&port->dev, "%s - endpoint %x (%d)\n",
 		__func__, usb_pipeendpoint(this_urb->pipe), device_port);
 
 	/* Save reset port val for resend.
-- 
2.7.3

[PATCH 6/8] USB: serial: mxuport: fix use-after-free in probe error path

[Johan Hovold]

The interface read and event URBs are submitted in attach, but were
never explicitly unlinked by the driver. Instead the URBs would have
been killed by usb-serial core on disconnect.

In case of a late probe error (e.g. due to failed minor allocation),
disconnect is never called and we could end up with active URBs for an
unbound interface. This in turn could lead to deallocated memory being
dereferenced in the completion callbacks.

Fixes: ee467a1f2066 ("USB: serial: add Moxa UPORT 12XX/14XX/16XX
driver")
Cc: stable <stable@vger.kernel.org>	# v3.14

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mxuport.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 31a8b47f1ac6..c6596cbcc4b6 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -1259,6 +1259,15 @@ static int mxuport_attach(struct usb_serial *serial)
 	return 0;
 }
 
+static void mxuport_release(struct usb_serial *serial)
+{
+	struct usb_serial_port *port0 = serial->port[0];
+	struct usb_serial_port *port1 = serial->port[1];
+
+	usb_serial_generic_close(port1);
+	usb_serial_generic_close(port0);
+}
+
 static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct mxuport_port *mxport = usb_get_serial_port_data(port);
@@ -1361,6 +1370,7 @@ static struct usb_serial_driver mxuport_device = {
 	.probe			= mxuport_probe,
 	.port_probe		= mxuport_port_probe,
 	.attach			= mxuport_attach,
+	.release		= mxuport_release,
 	.calc_num_ports		= mxuport_calc_num_ports,
 	.open			= mxuport_open,
 	.close			= mxuport_close,
-- 
2.7.3

[PATCH 7/8] USB: serial: quatech2: fix use-after-free in probe error path

[Johan Hovold]

The interface read URB is submitted in attach, but was only unlinked by
the driver at disconnect.

In case of a late probe error (e.g. due to failed minor allocation),
disconnect is never called and we would end up with active URBs for an
unbound interface. This in turn could lead to deallocated memory being
dereferenced in the completion callback.

Fixes: f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver")
Cc: stable <stable@vger.kernel.org>	# v3.5: 40d04738491d
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/quatech2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c
index 2df8ad5ede89..85acb50a7ee2 100644
--- a/drivers/usb/serial/quatech2.c
+++ b/drivers/usb/serial/quatech2.c
@@ -141,6 +141,7 @@ static void qt2_release(struct usb_serial *serial)
 
 	serial_priv = usb_get_serial_data(serial);
 
+	usb_kill_urb(serial_priv->read_urb);
 	usb_free_urb(serial_priv->read_urb);
 	kfree(serial_priv->read_buffer);
 	kfree(serial_priv);
-- 
2.7.3

[PATCH 8/8] USB: serial: fix minor-number allocation

[Johan Hovold]

Due to a missing upper bound, invalid minor numbers could be assigned to
ports. Such devices would later fail to register, but let's catch this
early as intended and avoid having devices with only a subset of their
ports registered (potentially the empty set).

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb-serial.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 7ecf4ff86b9a..4d2b310de55d 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -96,7 +96,8 @@ static int allocate_minors(struct usb_serial *serial, int num_ports)
 	mutex_lock(&table_lock);
 	for (i = 0; i < num_ports; ++i) {
 		port = serial->port[i];
-		minor = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
+		minor = idr_alloc(&serial_minors, port, 0,
+					USB_SERIAL_TTY_MINORS, GFP_KERNEL);
 		if (minor < 0)
 			goto error;
 		port->minor = minor;
-- 
2.7.3

Re: [PATCH 8/8] USB: serial: fix minor-number allocation

[Greg KH]

On Sun, May 08, 2016 at 08:08:03PM +0200, Johan Hovold wrote:
> Due to a missing upper bound, invalid minor numbers could be assigned to
> ports. Such devices would later fail to register, but let's catch this
> early as intended and avoid having devices with only a subset of their
> ports registered (potentially the empty set).
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/usb-serial.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index 7ecf4ff86b9a..4d2b310de55d 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -96,7 +96,8 @@ static int allocate_minors(struct usb_serial *serial, int num_ports)
>  	mutex_lock(&table_lock);
>  	for (i = 0; i < num_ports; ++i) {
>  		port = serial->port[i];
> -		minor = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> +		minor = idr_alloc(&serial_minors, port, 0,
> +					USB_SERIAL_TTY_MINORS, GFP_KERNEL);
>  		if (minor < 0)
>  			goto error;
>  		port->minor = minor;

Nice catch for this one, and all the others in this series.

Feel free to add:
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

to them all if you want.

thanks,

greg k-h

Re: [PATCH 8/8] USB: serial: fix minor-number allocation

[Johan Hovold]

On Mon, May 09, 2016 at 07:26:27AM +0200, Greg Kroah-Hartman wrote:
> On Sun, May 08, 2016 at 08:08:03PM +0200, Johan Hovold wrote:
> > Due to a missing upper bound, invalid minor numbers could be assigned to
> > ports. Such devices would later fail to register, but let's catch this
> > early as intended and avoid having devices with only a subset of their
> > ports registered (potentially the empty set).
> > 
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/usb/serial/usb-serial.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> > index 7ecf4ff86b9a..4d2b310de55d 100644
> > --- a/drivers/usb/serial/usb-serial.c
> > +++ b/drivers/usb/serial/usb-serial.c
> > @@ -96,7 +96,8 @@ static int allocate_minors(struct usb_serial *serial, int num_ports)
> >  	mutex_lock(&table_lock);
> >  	for (i = 0; i < num_ports; ++i) {
> >  		port = serial->port[i];
> > -		minor = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> > +		minor = idr_alloc(&serial_minors, port, 0,
> > +					USB_SERIAL_TTY_MINORS, GFP_KERNEL);
> >  		if (minor < 0)
> >  			goto error;
> >  		port->minor = minor;
> 
> Nice catch for this one, and all the others in this series.
> 
> Feel free to add:
> 	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> to them all if you want.

Thanks. All now applied.

Johan