USB: serial: cyberjack: fix NULL-deref at open
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Johan Hovold
maintainerPATCH 1/24 USB: serial: cyberjack: fix NULL-deref at open
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]
--- drivers/usb/serial/cyberjack.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/usb/serial/cyberjack.c +10 -0
@@ -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;
Signature · 1 line
2.10.2
Quoted 35 lines · 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 atcyberjack_open+0x40/0x9c[cyberjack]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Cc stable <[email protected]>Signed-off-by Johan Hovold <[email protected]>--- drivers/usb/serial/cyberjack.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/usb/serial/cyberjack.c +2
-0@@ -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,
drivers/usb/serial/cyberjack.c struct cyberjack_private { · line 111
Quoted 10 lines
drivers/usb/serial/cyberjack.c +7
-0@@ -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
Johan Hovold
maintainerQuoted 50 lines · Greg Kroah-Hartman
Quoted 35 lines · 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 atcyberjack_open+0x40/0x9c[cyberjack]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Cc stable <[email protected]>Signed-off-by Johan Hovold <[email protected]>--- drivers/usb/serial/cyberjack.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/usb/serial/cyberjack.c +2
-0@@ -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,drivers/usb/serial/cyberjack.c struct cyberjack_private { · line 111
Quoted 10 lines
drivers/usb/serial/cyberjack.c +7
-0@@ -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?
Quoted 5 lines
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).
Quoted 2 lines
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.
Quoted 2 lines
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
Quoted 57 lines · Johan Hovold
Quoted 50 lines · Greg Kroah-Hartman
Quoted 35 lines · 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 atcyberjack_open+0x40/0x9c[cyberjack]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Cc stable <[email protected]>Signed-off-by Johan Hovold <[email protected]>--- drivers/usb/serial/cyberjack.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/usb/serial/cyberjack.c +2
-0@@ -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,drivers/usb/serial/cyberjack.c struct cyberjack_private { · line 111
Quoted 10 lines
drivers/usb/serial/cyberjack.c +7
-0@@ -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?
Quoted 5 lines
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?
Quoted 7 lines
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-serialnum_endpointscheck")), 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.
Quoted 3 lines
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
Johan Hovold
maintainerPATCH 2/24 USB: serial: garmin_gps: fix memory leak on failed URB submit
In reply to a message not in the archiveMake sure to free the URB transfer buffer in case submission fails (e.g. due to a disconnect).
--- drivers/usb/serial/garmin_gps.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/serial/garmin_gps.c +1 -0
@@ -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
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 3/24 USB: serial: io_edgeport: fix NULL-deref at open
In reply to a message not in the archiveFix 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.
--- drivers/usb/serial/io_edgeport.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/usb/serial/io_edgeport.c +5 -0
@@ -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 */
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 4/24 USB: serial: io_ti: fix NULL-deref at open
In reply to a message not in the archiveFix 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)
--- drivers/usb/serial/io_ti.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/usb/serial/io_ti.c +7 -0
@@ -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)
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 5/24 USB: serial: io_ti: fix another NULL-deref at open
In reply to a message not in the archiveIn 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)
--- drivers/usb/serial/io_ti.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/usb/serial/io_ti.c +6 -3
@@ -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 ;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 6/24 USB: serial: io_ti: fix I/O after disconnect
In reply to a message not in the archiveCancel 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.
ports from disconnecting")
--- drivers/usb/serial/io_ti.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/serial/io_ti.c +3 -0
@@ -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)
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 8/24 USB: serial: iuu_phoenix: fix NULL-deref at open
In reply to a message not in the archiveFix 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]
check")
--- drivers/usb/serial/iuu_phoenix.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/usb/serial/iuu_phoenix.c +11 -0
@@ -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, };
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 9/24 USB: serial: keyspan_pda: verify endpoints at probe
In reply to a message not in the archiveCheck 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.
--- drivers/usb/serial/keyspan_pda.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
drivers/usb/serial/keyspan_pda.c +14 -0
@@ -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, };
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 10/24 USB: serial: kobil_sct: fix NULL-deref in write
In reply to a message not in the archiveFix 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]
--- drivers/usb/serial/kobil_sct.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/usb/serial/kobil_sct.c +12 -0
@@ -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;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 11/24 USB: serial: mos7720: fix NULL-deref at open
In reply to a message not in the archiveFix 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)
--- drivers/usb/serial/mos7720.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/usb/serial/mos7720.c +5 -0
@@ -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;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 12/24 USB: serial: mos7720: fix use-after-free on probe errors
In reply to a message not in the archiveThe 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)
port on moschip 7715")
--- drivers/usb/serial/mos7720.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/usb/serial/mos7720.c +5 -1
@@ -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 */
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 13/24 USB: serial: mos7720: fix parport use-after-free on probe errors
In reply to a message not in the archiveDo 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.
port on moschip 7715")
--- drivers/usb/serial/mos7720.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/usb/serial/mos7720.c +8 -10
@@ -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);
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 14/24 USB: serial: mos7720: fix parallel probe
In reply to a message not in the archiveA 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.
moschip 7715")
--- drivers/usb/serial/mos7720.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-)
drivers/usb/serial/mos7720.c +7 -23
@@ -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[] = {
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 16/24 USB: serial: mos7840: fix NULL-deref at open
In reply to a message not in the archiveFix 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.
--- drivers/usb/serial/mos7840.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/usb/serial/mos7840.c +12 -0
@@ -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,
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 19/24 USB: serial: omninet: fix NULL-derefs at open and disconnect
In reply to a message not in the archiveFix 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])
--- drivers/usb/serial/omninet.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/usb/serial/omninet.c +13 -0
@@ -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;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 20/24 USB: serial: oti6858: fix NULL-deref at open
In reply to a message not in the archiveFix 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.
cable)")
Cc: stable <[email protected]>
--- drivers/usb/serial/oti6858.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/usb/serial/oti6858.c +16 -0
@@ -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;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 21/24 USB: serial: pl2303: fix NULL-deref at open
In reply to a message not in the archiveFix 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.
--- drivers/usb/serial/pl2303.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/usb/serial/pl2303.c +8 -0
@@ -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 ;
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 22/24 USB: serial: quatech2: fix sleep-while-atomic in close
In reply to a message not in the archiveThe 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.
--- drivers/usb/serial/quatech2.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/usb/serial/quatech2.c +0 -4
@@ -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,
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 23/24 USB: serial: spcp8x5: fix NULL-deref at open
In reply to a message not in the archiveFix 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]
--- drivers/usb/serial/spcp8x5.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
drivers/usb/serial/spcp8x5.c +14 -0
@@ -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, };
Signature · 1 line
2.10.2
Johan Hovold
maintainerPATCH 24/24 USB: serial: ti_usb_3410_5052: fix NULL-deref at open
In reply to a message not in the archiveFix 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])
--- drivers/usb/serial/ti_usb_3410_5052.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/usb/serial/ti_usb_3410_5052.c +7 -0
@@ -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:
Signature · 1 line
2.10.2