[PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference when clearing halt at open should the device
lack a bulk-out endpoint.

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at cyberjack_open+0x40/0x9c [cyberjack]

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cyberjack.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
index 5f17a3b9916d..80260b08398b 100644
--- a/drivers/usb/serial/cyberjack.c
+++ b/drivers/usb/serial/cyberjack.c
@@ -50,6 +50,7 @@
 #define CYBERJACK_PRODUCT_ID	0x0100
 
 /* Function prototypes */
+static int cyberjack_attach(struct usb_serial *serial);
 static int cyberjack_port_probe(struct usb_serial_port *port);
 static int cyberjack_port_remove(struct usb_serial_port *port);
 static int  cyberjack_open(struct tty_struct *tty,
@@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
 	.description =		"Reiner SCT Cyberjack USB card reader",
 	.id_table =		id_table,
 	.num_ports =		1,
+	.attach =		cyberjack_attach,
 	.port_probe =		cyberjack_port_probe,
 	.port_remove =		cyberjack_port_remove,
 	.open =			cyberjack_open,
@@ -100,6 +102,14 @@ struct cyberjack_private {
 	short		wrsent;		/* Data already sent */
 };
 
+static int cyberjack_attach(struct usb_serial *serial)
+{
+	if (serial->num_bulk_out < serial->num_ports)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int cyberjack_port_probe(struct usb_serial_port *port)
 {
 	struct cyberjack_private *priv;
-- 
2.10.2

[PATCH 02/24] USB: serial: garmin_gps: fix memory leak on failed URB submit

[Johan Hovold]

Make sure to free the URB transfer buffer in case submission fails (e.g.
due to a disconnect).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/garmin_gps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 97cabf803c2f..b2f2e87aed94 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -1043,6 +1043,7 @@ static int garmin_write_bulk(struct usb_serial_port *port,
 		   "%s - usb_submit_urb(write bulk) failed with status = %d\n",
 				__func__, status);
 		count = status;
+		kfree(buffer);
 	}
 
 	/* we are done with this urb, so let the host driver
-- 
2.10.2

[PATCH 03/24] USB: serial: io_edgeport: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference when initialising URBs at open should a
non-EPIC device lack a bulk-in or interrupt-in endpoint.

Unable to handle kernel NULL pointer dereference at virtual address 00000028
...
PC is at edge_open+0x24c/0x3e8 [io_edgeport]

Note that the EPIC-device probe path has the required sanity checks so
this makes those checks partially redundant.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_edgeport.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index dcc0c58aaad5..d50e5773483f 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2751,6 +2751,11 @@ static int edge_startup(struct usb_serial *serial)
 					EDGE_COMPATIBILITY_MASK1,
 					EDGE_COMPATIBILITY_MASK2 };
 
+	if (serial->num_bulk_in < 1 || serial->num_interrupt_in < 1) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
 	dev = serial->dev;
 
 	/* create our private serial structure */
-- 
2.10.2

[PATCH 04/24] USB: serial: io_ti: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference when clearing halt at open should a
malicious device lack the expected endpoints when in download mode.

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
[<bf011ed8>] (edge_open [io_ti]) from [<bf000118>] (serial_port_activate+0x68/0x98 [usbserial])
[<bf000118>] (serial_port_activate [usbserial]) from [<c0470ca4>] (tty_port_open+0x9c/0xe8)
[<c0470ca4>] (tty_port_open) from [<bf000da0>] (serial_open+0x48/0x6c [usbserial])
[<bf000da0>] (serial_open [usbserial]) from [<c0469178>] (tty_open+0xcc/0x5cc)

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_ti.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index c339163698eb..bf8a4f432f4b 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2546,6 +2546,13 @@ static int edge_startup(struct usb_serial *serial)
 	int status;
 	u16 product_id;
 
+	/* Make sure we have the required endpoints when in download mode. */
+	if (serial->interface->cur_altsetting->desc.bNumEndpoints > 1) {
+		if (serial->num_bulk_in < serial->num_ports ||
+				serial->num_bulk_out < serial->num_ports)
+			return -ENODEV;
+	}
+
 	/* create our private serial structure */
 	edge_serial = kzalloc(sizeof(struct edgeport_serial), GFP_KERNEL);
 	if (!edge_serial)
-- 
2.10.2

[PATCH 05/24] USB: serial: io_ti: fix another NULL-deref at open

[Johan Hovold]

In case a device is left in "boot-mode" we must not register any port
devices in order to avoid a NULL-pointer dereference on open due to
missing endpoints. This could be used by a malicious device to trigger
an OOPS:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
[<bf0caa84>] (edge_open [io_ti]) from [<bf0b0118>] (serial_port_activate+0x68/0x98 [usbserial])
[<bf0b0118>] (serial_port_activate [usbserial]) from [<c0470ca4>] (tty_port_open+0x9c/0xe8)
[<c0470ca4>] (tty_port_open) from [<bf0b0da0>] (serial_open+0x48/0x6c [usbserial])
[<bf0b0da0>] (serial_open [usbserial]) from [<c0469178>] (tty_open+0xcc/0x5cc)

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_ti.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index bf8a4f432f4b..3b1cfba0ec84 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -1508,7 +1508,7 @@ static int do_boot_mode(struct edgeport_serial *serial,
 	dev_dbg(dev, "%s - STAYING IN BOOT MODE\n", __func__);
 	serial->product_info.TiMode = TI_MODE_BOOT;
 
-	return 0;
+	return 1;
 }
 
 static int ti_do_config(struct edgeport_port *port, int feature, int on)
@@ -2560,14 +2560,18 @@ static int edge_startup(struct usb_serial *serial)
 
 	mutex_init(&edge_serial->es_lock);
 	edge_serial->serial = serial;
+	INIT_DELAYED_WORK(&edge_serial->heartbeat_work, edge_heartbeat_work);
 	usb_set_serial_data(serial, edge_serial);
 
 	status = download_fw(edge_serial);
-	if (status) {
+	if (status < 0) {
 		kfree(edge_serial);
 		return status;
 	}
 
+	if (status > 0)
+		return 1;	/* bind but do not register any ports */
+
 	product_id = le16_to_cpu(
 			edge_serial->serial->dev->descriptor.idProduct);
 
@@ -2579,7 +2583,6 @@ static int edge_startup(struct usb_serial *serial)
 		}
 	}
 
-	INIT_DELAYED_WORK(&edge_serial->heartbeat_work, edge_heartbeat_work);
 	edge_heartbeat_schedule(edge_serial);
 
 	return 0;
-- 
2.10.2

[PATCH 06/24] USB: serial: io_ti: fix I/O after disconnect

[Johan Hovold]

Cancel the heartbeat work on driver unbind in order to avoid I/O after
disconnect in case the port is held open.

Note that the cancel in release() is still needed to stop the heartbeat
after late probe errors.

Fixes: 26c78daade0f ("USB: io_ti: Add heartbeat to keep idle EP/416
ports from disconnecting")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_ti.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 3b1cfba0ec84..4b0b978fb356 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2590,6 +2590,9 @@ static int edge_startup(struct usb_serial *serial)
 
 static void edge_disconnect(struct usb_serial *serial)
 {
+	struct edgeport_serial *edge_serial = usb_get_serial_data(serial);
+
+	cancel_delayed_work_sync(&edge_serial->heartbeat_work);
 }
 
 static void edge_release(struct usb_serial *serial)
-- 
2.10.2

[PATCH 08/24] USB: serial: iuu_phoenix: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference at open should the device lack a bulk-in or
bulk-out endpoint:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at iuu_open+0x78/0x59c [iuu_phoenix]

Fixes: 07c3b1a10016 ("USB: remove broken usb-serial num_endpoints
check")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/iuu_phoenix.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index 344b4eea4bd5..d57fb5199218 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -68,6 +68,16 @@ struct iuu_private {
 	u32 clk;
 };
 
+static int iuu_attach(struct usb_serial *serial)
+{
+	unsigned char num_ports = serial->num_ports;
+
+	if (serial->num_bulk_in < num_ports || serial->num_bulk_out < num_ports)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int iuu_port_probe(struct usb_serial_port *port)
 {
 	struct iuu_private *priv;
@@ -1196,6 +1206,7 @@ static struct usb_serial_driver iuu_device = {
 	.tiocmset = iuu_tiocmset,
 	.set_termios = iuu_set_termios,
 	.init_termios = iuu_init_termios,
+	.attach = iuu_attach,
 	.port_probe = iuu_port_probe,
 	.port_remove = iuu_port_remove,
 };
-- 
2.10.2

[PATCH 09/24] USB: serial: keyspan_pda: verify endpoints at probe

[Johan Hovold]

Check for the expected endpoints in attach() and fail loudly if not
present.

Note that failing to do this appears to be benign since da280e348866
("USB: keyspan_pda: clean up write-urb busy handling") which prevents a
NULL-pointer dereference in write() by never marking a non-existent
write-urb as free.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>	# < v3.3
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/keyspan_pda.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index e49ad0c63ad8..83523fcf6fb9 100644
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -699,6 +699,19 @@ MODULE_FIRMWARE("keyspan_pda/keyspan_pda.fw");
 MODULE_FIRMWARE("keyspan_pda/xircom_pgs.fw");
 #endif
 
+static int keyspan_pda_attach(struct usb_serial *serial)
+{
+	unsigned char num_ports = serial->num_ports;
+
+	if (serial->num_bulk_out < num_ports ||
+			serial->num_interrupt_in < num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int keyspan_pda_port_probe(struct usb_serial_port *port)
 {
 
@@ -776,6 +789,7 @@ static struct usb_serial_driver keyspan_pda_device = {
 	.break_ctl =		keyspan_pda_break_ctl,
 	.tiocmget =		keyspan_pda_tiocmget,
 	.tiocmset =		keyspan_pda_tiocmset,
+	.attach =		keyspan_pda_attach,
 	.port_probe =		keyspan_pda_port_probe,
 	.port_remove =		keyspan_pda_port_remove,
 };
-- 
2.10.2

[PATCH 10/24] USB: serial: kobil_sct: fix NULL-deref in write

[Johan Hovold]

Fix NULL-pointer dereference in write() should the device lack the
expected interrupt-out endpoint:

Unable to handle kernel NULL pointer dereference at virtual address 00000054
...
PC is at kobil_write+0x144/0x2a0 [kobil_sct]

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 2363654cafc9..813035f51fe7 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -51,6 +51,7 @@
 
 
 /* Function prototypes */
+static int kobil_attach(struct usb_serial *serial);
 static int kobil_port_probe(struct usb_serial_port *probe);
 static int kobil_port_remove(struct usb_serial_port *probe);
 static int  kobil_open(struct tty_struct *tty, struct usb_serial_port *port);
@@ -86,6 +87,7 @@ static struct usb_serial_driver kobil_device = {
 	.description =		"KOBIL USB smart card terminal",
 	.id_table =		id_table,
 	.num_ports =		1,
+	.attach =		kobil_attach,
 	.port_probe =		kobil_port_probe,
 	.port_remove =		kobil_port_remove,
 	.ioctl =		kobil_ioctl,
@@ -113,6 +115,16 @@ struct kobil_private {
 };
 
 
+static int kobil_attach(struct usb_serial *serial)
+{
+	if (serial->num_interrupt_out < serial->num_ports) {
+		dev_err(&serial->interface->dev, "missing interrupt-out endpoint\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int kobil_port_probe(struct usb_serial_port *port)
 {
 	struct usb_serial *serial = port->serial;
-- 
2.10.2

[PATCH 11/24] USB: serial: mos7720: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference at port open if a device lacks the expected
bulk in and out endpoints.

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
[<bf071c20>] (mos7720_open [mos7720]) from [<bf0490e0>] (serial_port_activate+0x68/0x98 [usbserial])
[<bf0490e0>] (serial_port_activate [usbserial]) from [<c0470ca4>] (tty_port_open+0x9c/0xe8)
[<c0470ca4>] (tty_port_open) from [<bf049d98>] (serial_open+0x48/0x6c [usbserial])
[<bf049d98>] (serial_open [usbserial]) from [<c0469178>] (tty_open+0xcc/0x5cc)

Fixes: 0f64478cbc7a ("USB: add USB serial mos7720 driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mos7720.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index d52caa03679c..3220d0bc767b 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1917,6 +1917,11 @@ static int mos7720_startup(struct usb_serial *serial)
 	u16 product;
 	int ret_val;
 
+	if (serial->num_bulk_in < 2 || serial->num_bulk_out < 2) {
+		dev_err(&serial->interface->dev, "missing bulk endpoints\n");
+		return -ENODEV;
+	}
+
 	product = le16_to_cpu(serial->dev->descriptor.idProduct);
 	dev = serial->dev;
 
-- 
2.10.2

[PATCH 12/24] USB: serial: mos7720: fix use-after-free on probe errors

[Johan Hovold]

The interrupt URB was submitted on probe but never stopped on probe
errors. This can lead to use-after-free issues in the completion
handler when accessing the freed usb-serial struct:

Unable to handle kernel paging request at virtual address 6b6b6be7
...
[<bf052e70>] (mos7715_interrupt_callback [mos7720]) from [<c052a894>] (__usb_hcd_giveback_urb+0x80/0x140)
[<c052a894>] (__usb_hcd_giveback_urb) from [<c052a9a4>] (usb_hcd_giveback_urb+0x50/0x138)
[<c052a9a4>] (usb_hcd_giveback_urb) from [<c0550684>] (musb_giveback+0xc8/0x1cc)

Fixes: b69578df7e98 ("USB: usbserial: mos7720: add support for parallel
port on moschip 7715")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mos7720.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 3220d0bc767b..9170ae856b34 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1962,8 +1962,10 @@ static int mos7720_startup(struct usb_serial *serial)
 #ifdef CONFIG_USB_SERIAL_MOS7715_PARPORT
 	if (product == MOSCHIP_DEVICE_ID_7715) {
 		ret_val = mos7715_parport_init(serial);
-		if (ret_val < 0)
+		if (ret_val < 0) {
+			usb_kill_urb(serial->port[0]->interrupt_in_urb);
 			return ret_val;
+		}
 	}
 #endif
 	/* LSR For Port 1 */
@@ -1975,6 +1977,8 @@ static int mos7720_startup(struct usb_serial *serial)
 
 static void mos7720_release(struct usb_serial *serial)
 {
+	usb_kill_urb(serial->port[0]->interrupt_in_urb);
+
 #ifdef CONFIG_USB_SERIAL_MOS7715_PARPORT
 	/* close the parallel port */
 
-- 
2.10.2

[PATCH 13/24] USB: serial: mos7720: fix parport use-after-free on probe errors

[Johan Hovold]

Do not submit the interrupt URB until after the parport has been
successfully registered to avoid another use-after-free in the
completion handler when accessing the freed parport private data in case
of a racing completion.

Fixes: b69578df7e98 ("USB: usbserial: mos7720: add support for parallel
port on moschip 7715")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mos7720.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 9170ae856b34..e494821ae528 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1952,22 +1952,20 @@ static int mos7720_startup(struct usb_serial *serial)
 	usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
 			(__u8)0x03, 0x00, 0x01, 0x00, NULL, 0x00, 5000);
 
-	/* start the interrupt urb */
-	ret_val = usb_submit_urb(serial->port[0]->interrupt_in_urb, GFP_KERNEL);
-	if (ret_val)
-		dev_err(&dev->dev,
-			"%s - Error %d submitting control urb\n",
-			__func__, ret_val);
-
 #ifdef CONFIG_USB_SERIAL_MOS7715_PARPORT
 	if (product == MOSCHIP_DEVICE_ID_7715) {
 		ret_val = mos7715_parport_init(serial);
-		if (ret_val < 0) {
-			usb_kill_urb(serial->port[0]->interrupt_in_urb);
+		if (ret_val < 0)
 			return ret_val;
-		}
 	}
 #endif
+	/* start the interrupt urb */
+	ret_val = usb_submit_urb(serial->port[0]->interrupt_in_urb, GFP_KERNEL);
+	if (ret_val) {
+		dev_err(&dev->dev, "failed to submit interrupt urb: %d\n",
+			ret_val);
+	}
+
 	/* LSR For Port 1 */
 	read_mos_reg(serial, 0, MOS7720_LSR, &data);
 	dev_dbg(&dev->dev, "LSR:%x\n", data);
-- 
2.10.2

[PATCH 14/24] USB: serial: mos7720: fix parallel probe

[Johan Hovold]

A static usb-serial-driver structure that is used to initialise the
interrupt URB was modified during probe depending on the currently
probed device type, something which could break a parallel probe of a
device of a different type.

Fix this up by overriding the default completion callback for MCS7715
devices in attach() instead. We may want to use two usb-serial driver
instances for the two types later.

Fixes: fb088e335d78 ("USB: serial: add support for serial port on the
moschip 7715")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mos7720.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index e494821ae528..d6c4441db5f8 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -65,8 +65,6 @@ struct moschip_port {
 	struct urb		*write_urb_pool[NUM_URBS];
 };
 
-static struct usb_serial_driver moschip7720_2port_driver;
-
 #define USB_VENDOR_ID_MOSCHIP		0x9710
 #define MOSCHIP_DEVICE_ID_7720		0x7720
 #define MOSCHIP_DEVICE_ID_7715		0x7715
@@ -970,25 +968,6 @@ static void mos7720_bulk_out_data_callback(struct urb *urb)
 		tty_port_tty_wakeup(&mos7720_port->port->port);
 }
 
-/*
- * mos77xx_probe
- *	this function installs the appropriate read interrupt endpoint callback
- *	depending on whether the device is a 7720 or 7715, thus avoiding costly
- *	run-time checks in the high-frequency callback routine itself.
- */
-static int mos77xx_probe(struct usb_serial *serial,
-			 const struct usb_device_id *id)
-{
-	if (id->idProduct == MOSCHIP_DEVICE_ID_7715)
-		moschip7720_2port_driver.read_int_callback =
-			mos7715_interrupt_callback;
-	else
-		moschip7720_2port_driver.read_int_callback =
-			mos7720_interrupt_callback;
-
-	return 0;
-}
-
 static int mos77xx_calc_num_ports(struct usb_serial *serial)
 {
 	u16 product = le16_to_cpu(serial->dev->descriptor.idProduct);
@@ -1946,6 +1925,12 @@ static int mos7720_startup(struct usb_serial *serial)
 			tmp->interrupt_in_endpointAddress;
 		serial->port[1]->interrupt_in_urb = NULL;
 		serial->port[1]->interrupt_in_buffer = NULL;
+
+		if (serial->port[0]->interrupt_in_urb) {
+			struct urb *urb = serial->port[0]->interrupt_in_urb;
+
+			urb->complete = mos7715_interrupt_callback;
+		}
 	}
 
 	/* setting configuration feature to one */
@@ -2060,7 +2045,6 @@ static struct usb_serial_driver moschip7720_2port_driver = {
 	.close			= mos7720_close,
 	.throttle		= mos7720_throttle,
 	.unthrottle		= mos7720_unthrottle,
-	.probe			= mos77xx_probe,
 	.attach			= mos7720_startup,
 	.release		= mos7720_release,
 	.port_probe		= mos7720_port_probe,
@@ -2074,7 +2058,7 @@ static struct usb_serial_driver moschip7720_2port_driver = {
 	.chars_in_buffer	= mos7720_chars_in_buffer,
 	.break_ctl		= mos7720_break,
 	.read_bulk_callback	= mos7720_bulk_in_callback,
-	.read_int_callback	= NULL  /* dynamically assigned in probe() */
+	.read_int_callback	= mos7720_interrupt_callback,
 };
 
 static struct usb_serial_driver * const serial_drivers[] = {
-- 
2.10.2

[PATCH 16/24] USB: serial: mos7840: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference in open() should the device lack the
expected endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at mos7840_open+0x88/0x8dc [mos7840]

Note that we continue to treat the interrupt-in endpoint as optional for
now.

Fixes: 3f5429746d91 ("USB: Moschip 7840 USB-Serial Driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/mos7840.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 9a220b8e810f..bb933c6321e5 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -2113,6 +2113,17 @@ static int mos7840_calc_num_ports(struct usb_serial *serial)
 	return mos7840_num_ports;
 }
 
+static int mos7840_attach(struct usb_serial *serial)
+{
+	if (serial->num_bulk_in < serial->num_ports ||
+			serial->num_bulk_out < serial->num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int mos7840_port_probe(struct usb_serial_port *port)
 {
 	struct usb_serial *serial = port->serial;
@@ -2388,6 +2399,7 @@ static struct usb_serial_driver moschip7840_4port_device = {
 	.tiocmset = mos7840_tiocmset,
 	.tiocmiwait = usb_serial_generic_tiocmiwait,
 	.get_icount = usb_serial_generic_get_icount,
+	.attach = mos7840_attach,
 	.port_probe = mos7840_port_probe,
 	.port_remove = mos7840_port_remove,
 	.read_bulk_callback = mos7840_bulk_in_callback,
-- 
2.10.2

[PATCH 19/24] USB: serial: omninet: fix NULL-derefs at open and disconnect

[Johan Hovold]

Fix NULL-pointer dereferences at open() and disconnect() should the
device lack the expected bulk-out endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 000000b4
...
[c0170ff0>] (__lock_acquire) from [<c0172f00>] (lock_acquire+0x108/0x264)
[<c0172f00>] (lock_acquire) from [<c06a5090>] (_raw_spin_lock_irqsave+0x58/0x6c)
[<c06a5090>] (_raw_spin_lock_irqsave) from [<c0470684>] (tty_port_tty_set+0x28/0xa4)
[<c0470684>] (tty_port_tty_set) from [<bf08d384>] (omninet_open+0x30/0x40 [omninet])
[<bf08d384>] (omninet_open [omninet]) from [<bf07c118>] (serial_port_activate+0x68/0x98 [usbserial])

Unable to handle kernel NULL pointer dereference at virtual address 00000234
...
[<bf01f418>] (omninet_disconnect [omninet]) from [<bf0016c0>] (usb_serial_disconnect+0xe4/0x100 [usbserial])

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/omninet.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c
index f6c6900bccf0..a180b17d2432 100644
--- a/drivers/usb/serial/omninet.c
+++ b/drivers/usb/serial/omninet.c
@@ -38,6 +38,7 @@ static int  omninet_write(struct tty_struct *tty, struct usb_serial_port *port,
 				const unsigned char *buf, int count);
 static int  omninet_write_room(struct tty_struct *tty);
 static void omninet_disconnect(struct usb_serial *serial);
+static int omninet_attach(struct usb_serial *serial);
 static int omninet_port_probe(struct usb_serial_port *port);
 static int omninet_port_remove(struct usb_serial_port *port);
 
@@ -56,6 +57,7 @@ static struct usb_serial_driver zyxel_omninet_device = {
 	.description =		"ZyXEL - omni.net lcd plus usb",
 	.id_table =		id_table,
 	.num_ports =		1,
+	.attach =		omninet_attach,
 	.port_probe =		omninet_port_probe,
 	.port_remove =		omninet_port_remove,
 	.open =			omninet_open,
@@ -104,6 +106,17 @@ struct omninet_data {
 	__u8	od_outseq;	/* Sequence number for bulk_out URBs */
 };
 
+static int omninet_attach(struct usb_serial *serial)
+{
+	/* The second bulk-out endpoint is used for writing. */
+	if (serial->num_bulk_out < 2) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int omninet_port_probe(struct usb_serial_port *port)
 {
 	struct omninet_data *od;
-- 
2.10.2

[PATCH 20/24] USB: serial: oti6858: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference in open() should the device lack the
expected endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at oti6858_open+0x30/0x1d0 [oti6858]

Note that a missing interrupt-in endpoint would have caused open() to
fail.

Fixes: 49cdee0ed0fc ("USB: oti6858 usb-serial driver (in Nokia CA-42
cable)")
Cc: stable <stable@vger.kernel.org>

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

diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c
index a4b88bc038b6..b8bf52bf7a94 100644
--- a/drivers/usb/serial/oti6858.c
+++ b/drivers/usb/serial/oti6858.c
@@ -134,6 +134,7 @@ static int oti6858_chars_in_buffer(struct tty_struct *tty);
 static int oti6858_tiocmget(struct tty_struct *tty);
 static int oti6858_tiocmset(struct tty_struct *tty,
 				unsigned int set, unsigned int clear);
+static int oti6858_attach(struct usb_serial *serial);
 static int oti6858_port_probe(struct usb_serial_port *port);
 static int oti6858_port_remove(struct usb_serial_port *port);
 
@@ -158,6 +159,7 @@ static struct usb_serial_driver oti6858_device = {
 	.write_bulk_callback =	oti6858_write_bulk_callback,
 	.write_room =		oti6858_write_room,
 	.chars_in_buffer =	oti6858_chars_in_buffer,
+	.attach =		oti6858_attach,
 	.port_probe =		oti6858_port_probe,
 	.port_remove =		oti6858_port_remove,
 };
@@ -324,6 +326,20 @@ static void send_data(struct work_struct *work)
 	usb_serial_port_softint(port);
 }
 
+static int oti6858_attach(struct usb_serial *serial)
+{
+	unsigned char num_ports = serial->num_ports;
+
+	if (serial->num_bulk_in < num_ports ||
+			serial->num_bulk_out < num_ports ||
+			serial->num_interrupt_in < num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int oti6858_port_probe(struct usb_serial_port *port)
 {
 	struct oti6858_private *priv;
-- 
2.10.2

[PATCH 21/24] USB: serial: pl2303: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference in open() should a type-0 or type-1 device
lack the expected endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at pl2303_open+0x38/0xec [pl2303]

Note that a missing interrupt-in endpoint would have caused open() to
fail.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/pl2303.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index ae682e4eeaef..46fca6b75846 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -220,9 +220,17 @@ static int pl2303_probe(struct usb_serial *serial,
 static int pl2303_startup(struct usb_serial *serial)
 {
 	struct pl2303_serial_private *spriv;
+	unsigned char num_ports = serial->num_ports;
 	enum pl2303_type type = TYPE_01;
 	unsigned char *buf;
 
+	if (serial->num_bulk_in < num_ports ||
+			serial->num_bulk_out < num_ports ||
+			serial->num_interrupt_in < num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
 	spriv = kzalloc(sizeof(*spriv), GFP_KERNEL);
 	if (!spriv)
 		return -ENOMEM;
-- 
2.10.2

[PATCH 22/24] USB: serial: quatech2: fix sleep-while-atomic in close

[Johan Hovold]

The write URB was being killed using the synchronous interface while
holding a spin lock in close().

Simply drop the lock and busy-flag update, something which would have
been taken care of by the completion handler if the URB was in flight.

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

diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c
index 659cb8606bd9..5709cc93b083 100644
--- a/drivers/usb/serial/quatech2.c
+++ b/drivers/usb/serial/quatech2.c
@@ -408,16 +408,12 @@ static void qt2_close(struct usb_serial_port *port)
 {
 	struct usb_serial *serial;
 	struct qt2_port_private *port_priv;
-	unsigned long flags;
 	int i;
 
 	serial = port->serial;
 	port_priv = usb_get_serial_port_data(port);
 
-	spin_lock_irqsave(&port_priv->urb_lock, flags);
 	usb_kill_urb(port_priv->write_urb);
-	port_priv->urb_in_use = false;
-	spin_unlock_irqrestore(&port_priv->urb_lock, flags);
 
 	/* flush the port transmit buffer */
 	i = usb_control_msg(serial->dev,
-- 
2.10.2

[PATCH 23/24] USB: serial: spcp8x5: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference in open() should the device lack the
expected endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
...
PC is at spcp8x5_open+0x30/0xd0 [spcp8x5]

Fixes: 619a6f1d1423 ("USB: add usb-serial spcp8x5 driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/spcp8x5.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/serial/spcp8x5.c b/drivers/usb/serial/spcp8x5.c
index ef0dbf0703c5..475e6c31b266 100644
--- a/drivers/usb/serial/spcp8x5.c
+++ b/drivers/usb/serial/spcp8x5.c
@@ -154,6 +154,19 @@ static int spcp8x5_probe(struct usb_serial *serial,
 	return 0;
 }
 
+static int spcp8x5_attach(struct usb_serial *serial)
+{
+	unsigned char num_ports = serial->num_ports;
+
+	if (serial->num_bulk_in < num_ports ||
+			serial->num_bulk_out < num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int spcp8x5_port_probe(struct usb_serial_port *port)
 {
 	const struct usb_device_id *id = usb_get_serial_data(port->serial);
@@ -477,6 +490,7 @@ static struct usb_serial_driver spcp8x5_device = {
 	.tiocmget		= spcp8x5_tiocmget,
 	.tiocmset		= spcp8x5_tiocmset,
 	.probe			= spcp8x5_probe,
+	.attach			= spcp8x5_attach,
 	.port_probe		= spcp8x5_port_probe,
 	.port_remove		= spcp8x5_port_remove,
 };
-- 
2.10.2

[PATCH 24/24] USB: serial: ti_usb_3410_5052: fix NULL-deref at open

[Johan Hovold]

Fix NULL-pointer dereference in open() should a malicious device lack
the expected endpoints:

Unable to handle kernel NULL pointer dereference at virtual address 00000030
..
[<bf06a6b0>] (ti_open [ti_usb_3410_5052]) from [<bf02e118>] (serial_port_activate+0x68/0x98 [usbserial])

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 8db9d071d940..64b85b8dedf3 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -579,6 +579,13 @@ static int ti_startup(struct usb_serial *serial)
 		goto free_tdev;
 	}
 
+	if (serial->num_bulk_in < serial->num_ports ||
+			serial->num_bulk_out < serial->num_ports) {
+		dev_err(&serial->interface->dev, "missing endpoints\n");
+		status = -ENODEV;
+		goto free_tdev;
+	}
+
 	return 0;
 
 free_tdev:
-- 
2.10.2

Re: [PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[Greg KH]

On Tue, Jan 03, 2017 at 04:39:40PM +0100, Johan Hovold wrote:
> Fix NULL-pointer dereference when clearing halt at open should the device
> lack a bulk-out endpoint.
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000030
> ...
> PC is at cyberjack_open+0x40/0x9c [cyberjack]
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/cyberjack.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
> index 5f17a3b9916d..80260b08398b 100644
> --- a/drivers/usb/serial/cyberjack.c
> +++ b/drivers/usb/serial/cyberjack.c
> @@ -50,6 +50,7 @@
>  #define CYBERJACK_PRODUCT_ID	0x0100
>  
>  /* Function prototypes */
> +static int cyberjack_attach(struct usb_serial *serial);
>  static int cyberjack_port_probe(struct usb_serial_port *port);
>  static int cyberjack_port_remove(struct usb_serial_port *port);
>  static int  cyberjack_open(struct tty_struct *tty,
> @@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
>  	.description =		"Reiner SCT Cyberjack USB card reader",
>  	.id_table =		id_table,
>  	.num_ports =		1,
> +	.attach =		cyberjack_attach,
>  	.port_probe =		cyberjack_port_probe,
>  	.port_remove =		cyberjack_port_remove,
>  	.open =			cyberjack_open,
> @@ -100,6 +102,14 @@ struct cyberjack_private {
>  	short		wrsent;		/* Data already sent */
>  };
>  
> +static int cyberjack_attach(struct usb_serial *serial)
> +{
> +	if (serial->num_bulk_out < serial->num_ports)
> +		return -ENODEV;
> +
> +	return 0;
> +}

You end up doing much the same thing for most of these drivers, is there
any way to do it in the usb-serial core instead?

I've been playing with an idea to have a USB driver specify the number
and types of endpoints it requires and have the core just not even call
the probe function if that doesn't match up.  That should solve lots of
these issues, can't you do much the same type of thing here instead of
requiring a callback to do this?

Hm, but you want to match up the number of ports with the number of
bulk endpoint pairs.  That's tricky...

Anyway, I guess this is ok, I just get worried when I see a bunch of the
same changes in a bunch of different drivers.

thanks,

greg k-h

Re: [PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[Johan Hovold]

On Tue, Jan 03, 2017 at 05:27:07PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 03, 2017 at 04:39:40PM +0100, Johan Hovold wrote:
> > Fix NULL-pointer dereference when clearing halt at open should the device
> > lack a bulk-out endpoint.
> > 
> > Unable to handle kernel NULL pointer dereference at virtual address 00000030
> > ...
> > PC is at cyberjack_open+0x40/0x9c [cyberjack]
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable <stable@vger.kernel.org>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/usb/serial/cyberjack.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
> > index 5f17a3b9916d..80260b08398b 100644
> > --- a/drivers/usb/serial/cyberjack.c
> > +++ b/drivers/usb/serial/cyberjack.c
> > @@ -50,6 +50,7 @@
> >  #define CYBERJACK_PRODUCT_ID	0x0100
> >  
> >  /* Function prototypes */
> > +static int cyberjack_attach(struct usb_serial *serial);
> >  static int cyberjack_port_probe(struct usb_serial_port *port);
> >  static int cyberjack_port_remove(struct usb_serial_port *port);
> >  static int  cyberjack_open(struct tty_struct *tty,
> > @@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
> >  	.description =		"Reiner SCT Cyberjack USB card reader",
> >  	.id_table =		id_table,
> >  	.num_ports =		1,
> > +	.attach =		cyberjack_attach,
> >  	.port_probe =		cyberjack_port_probe,
> >  	.port_remove =		cyberjack_port_remove,
> >  	.open =			cyberjack_open,
> > @@ -100,6 +102,14 @@ struct cyberjack_private {
> >  	short		wrsent;		/* Data already sent */
> >  };
> >  
> > +static int cyberjack_attach(struct usb_serial *serial)
> > +{
> > +	if (serial->num_bulk_out < serial->num_ports)
> > +		return -ENODEV;
> > +
> > +	return 0;
> > +}
> 
> You end up doing much the same thing for most of these drivers, is there
> any way to do it in the usb-serial core instead?
> 
> I've been playing with an idea to have a USB driver specify the number
> and types of endpoints it requires and have the core just not even call
> the probe function if that doesn't match up.  That should solve lots of
> these issues, can't you do much the same type of thing here instead of
> requiring a callback to do this?

I've been playing with that same idea, but I wanted minimal fixes that
could be backported to the stable trees for this first. I also kept the
checks as loose as possible to avoid any regressions.

Note that there seems to have been a general mechanism for this that was
removed in 2008 (see 07c3b1a10016 ("USB: remove broken usb-serial
num_endpoints check")), possibly because the checks were too tight.

But since there appear to be exploits out there for this class of
issues, we should probably consider reintroducing it in some form (e.g.
in USB core or USB serial core).

> Hm, but you want to match up the number of ports with the number of
> bulk endpoint pairs.  That's tricky...

Yeah, and that's very much device dependent too, and layouts can change
after firmware is loaded, etc.

> Anyway, I guess this is ok, I just get worried when I see a bunch of the
> same changes in a bunch of different drivers.

Thanks,
Johan

Re: [PATCH 01/24] USB: serial: cyberjack: fix NULL-deref at open

[Greg KH]

On Tue, Jan 03, 2017 at 05:48:05PM +0100, Johan Hovold wrote:
> On Tue, Jan 03, 2017 at 05:27:07PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 03, 2017 at 04:39:40PM +0100, Johan Hovold wrote:
> > > Fix NULL-pointer dereference when clearing halt at open should the device
> > > lack a bulk-out endpoint.
> > > 
> > > Unable to handle kernel NULL pointer dereference at virtual address 00000030
> > > ...
> > > PC is at cyberjack_open+0x40/0x9c [cyberjack]
> > > 
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Cc: stable <stable@vger.kernel.org>
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  drivers/usb/serial/cyberjack.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
> > > index 5f17a3b9916d..80260b08398b 100644
> > > --- a/drivers/usb/serial/cyberjack.c
> > > +++ b/drivers/usb/serial/cyberjack.c
> > > @@ -50,6 +50,7 @@
> > >  #define CYBERJACK_PRODUCT_ID	0x0100
> > >  
> > >  /* Function prototypes */
> > > +static int cyberjack_attach(struct usb_serial *serial);
> > >  static int cyberjack_port_probe(struct usb_serial_port *port);
> > >  static int cyberjack_port_remove(struct usb_serial_port *port);
> > >  static int  cyberjack_open(struct tty_struct *tty,
> > > @@ -77,6 +78,7 @@ static struct usb_serial_driver cyberjack_device = {
> > >  	.description =		"Reiner SCT Cyberjack USB card reader",
> > >  	.id_table =		id_table,
> > >  	.num_ports =		1,
> > > +	.attach =		cyberjack_attach,
> > >  	.port_probe =		cyberjack_port_probe,
> > >  	.port_remove =		cyberjack_port_remove,
> > >  	.open =			cyberjack_open,
> > > @@ -100,6 +102,14 @@ struct cyberjack_private {
> > >  	short		wrsent;		/* Data already sent */
> > >  };
> > >  
> > > +static int cyberjack_attach(struct usb_serial *serial)
> > > +{
> > > +	if (serial->num_bulk_out < serial->num_ports)
> > > +		return -ENODEV;
> > > +
> > > +	return 0;
> > > +}
> > 
> > You end up doing much the same thing for most of these drivers, is there
> > any way to do it in the usb-serial core instead?
> > 
> > I've been playing with an idea to have a USB driver specify the number
> > and types of endpoints it requires and have the core just not even call
> > the probe function if that doesn't match up.  That should solve lots of
> > these issues, can't you do much the same type of thing here instead of
> > requiring a callback to do this?
> 
> I've been playing with that same idea, but I wanted minimal fixes that
> could be backported to the stable trees for this first. I also kept the
> checks as loose as possible to avoid any regressions.
> 
> Note that there seems to have been a general mechanism for this that was
> removed in 2008 (see 07c3b1a10016 ("USB: remove broken usb-serial
> num_endpoints check")), possibly because the checks were too tight.

I have no idea why I wrote that patch, but I guess things were failing
to bind for devices that were valid.

> But since there appear to be exploits out there for this class of
> issues, we should probably consider reintroducing it in some form (e.g.
> in USB core or USB serial core).

As the usb-serial core has to take all types of devices, I think it will
have to go into both :(

Your patches are good, and you are right, they will work well for stable
kernels, so we should take them now.  I'll work on the usb core option
in the next few weeks after I catch up from my vacation patch backlog,
so that's a nicer long-term option for where we can make the drivers do
less work and remove code when that happens.

thanks,

greg k-h