From 2e46565cf622dd0534a9d8bffe152a577b48d7aa Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 24 Feb 2017 19:11:28 +0100 Subject: [PATCH 1/6] USB: serial: digi_acceleport: fix OOB-event processing A recent change claimed to fix an off-by-one error in the OOB-port completion handler, but instead introduced such an error. This could specifically led to modem-status changes going unnoticed, effectively breaking TIOCMGET. Note that the offending commit fixes a loop-condition underflow and is marked for stable, but should not be backported without this fix. Reported-by: Ben Hutchings Fixes: 2d380889215f ("USB: serial: digi_acceleport: fix OOB data sanity check") Cc: stable # v2.6.30: 2d380889215f Signed-off-by: Johan Hovold --- drivers/usb/serial/digi_acceleport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c index ab78111e0968..6537d3ca2797 100644 --- a/drivers/usb/serial/digi_acceleport.c +++ b/drivers/usb/serial/digi_acceleport.c @@ -1500,7 +1500,7 @@ static int digi_read_oob_callback(struct urb *urb) return -1; /* handle each oob command */ - for (i = 0; i < urb->actual_length - 4; i += 4) { + for (i = 0; i < urb->actual_length - 3; i += 4) { opcode = buf[i]; line = buf[i + 1]; status = buf[i + 2]; From 0b1d250afb8eb9d65afb568bac9b9f9253a82b49 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 6 Mar 2017 17:36:37 +0100 Subject: [PATCH 2/6] USB: serial: io_ti: fix NULL-deref in interrupt callback Fix a NULL-pointer dereference in the interrupt callback should a malicious device send data containing a bad port number by adding the missing sanity check. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/io_ti.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c index ceaeebaa6f90..4561dd4cde8b 100644 --- a/drivers/usb/serial/io_ti.c +++ b/drivers/usb/serial/io_ti.c @@ -1674,6 +1674,12 @@ static void edge_interrupt_callback(struct urb *urb) function = TIUMP_GET_FUNC_FROM_CODE(data[0]); dev_dbg(dev, "%s - port_number %d, function %d, info 0x%x\n", __func__, port_number, function, data[1]); + + if (port_number >= edge_serial->serial->num_ports) { + dev_err(dev, "bad port number %d\n", port_number); + goto exit; + } + port = edge_serial->serial->port[port_number]; edge_port = usb_get_serial_port_data(port); if (!edge_port) { From 30572418b445d85fcfe6c8fe84c947d2606767d8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 6 Mar 2017 17:36:38 +0100 Subject: [PATCH 3/6] USB: serial: omninet: fix reference leaks at open This driver needlessly took another reference to the tty on open, a reference which was then never released on close. This lead to not just a leak of the tty, but also a driver reference leak that prevented the driver from being unloaded after a port had once been opened. Fixes: 4a90f09b20f4 ("tty: usb-serial krefs") Cc: stable # 2.6.28 Signed-off-by: Johan Hovold --- drivers/usb/serial/omninet.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c index a180b17d2432..76564b3bebb9 100644 --- a/drivers/usb/serial/omninet.c +++ b/drivers/usb/serial/omninet.c @@ -142,12 +142,6 @@ static int omninet_port_remove(struct usb_serial_port *port) static int omninet_open(struct tty_struct *tty, struct usb_serial_port *port) { - struct usb_serial *serial = port->serial; - struct usb_serial_port *wport; - - wport = serial->port[1]; - tty_port_tty_set(&wport->port, tty); - return usb_serial_generic_open(tty, port); } From 367ec1706745912702c187722065285cd4d2aee7 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 6 Mar 2017 17:36:39 +0100 Subject: [PATCH 4/6] USB: serial: omninet: drop open callback Remove the now redundant open callback and let core call the generic handler for us instead. Signed-off-by: Johan Hovold --- drivers/usb/serial/omninet.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c index 76564b3bebb9..dd706953b466 100644 --- a/drivers/usb/serial/omninet.c +++ b/drivers/usb/serial/omninet.c @@ -31,7 +31,6 @@ #define BT_IGNITIONPRO_ID 0x2000 /* function prototypes */ -static int omninet_open(struct tty_struct *tty, struct usb_serial_port *port); static void omninet_process_read_urb(struct urb *urb); static void omninet_write_bulk_callback(struct urb *urb); static int omninet_write(struct tty_struct *tty, struct usb_serial_port *port, @@ -60,7 +59,6 @@ static struct usb_serial_driver zyxel_omninet_device = { .attach = omninet_attach, .port_probe = omninet_port_probe, .port_remove = omninet_port_remove, - .open = omninet_open, .write = omninet_write, .write_room = omninet_write_room, .write_bulk_callback = omninet_write_bulk_callback, @@ -140,11 +138,6 @@ static int omninet_port_remove(struct usb_serial_port *port) return 0; } -static int omninet_open(struct tty_struct *tty, struct usb_serial_port *port) -{ - return usb_serial_generic_open(tty, port); -} - #define OMNINET_HEADERLEN 4 #define OMNINET_BULKOUTSIZE 64 #define OMNINET_PAYLOADSIZE (OMNINET_BULKOUTSIZE - OMNINET_HEADERLEN) From 654b404f2a222f918af9b0cd18ad469d0c941a8e Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 6 Mar 2017 17:36:40 +0100 Subject: [PATCH 5/6] USB: serial: io_ti: fix information leak in completion handler Add missing sanity check to the bulk-in completion handler to avoid an integer underflow that can be triggered by a malicious device. This avoids leaking 128 kB of memory content from after the URB transfer buffer to user space. Fixes: 8c209e6782ca ("USB: make actual_length in struct urb field u32") Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable # 2.6.30 Signed-off-by: Johan Hovold --- drivers/usb/serial/io_ti.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c index 4561dd4cde8b..a76b95d32157 100644 --- a/drivers/usb/serial/io_ti.c +++ b/drivers/usb/serial/io_ti.c @@ -1761,7 +1761,7 @@ static void edge_bulk_in_callback(struct urb *urb) port_number = edge_port->port->port_number; - if (edge_port->lsr_event) { + if (urb->actual_length > 0 && edge_port->lsr_event) { edge_port->lsr_event = 0; dev_dbg(dev, "%s ===== Port %u LSR Status = %02x, Data = %02x ======\n", __func__, port_number, edge_port->lsr_mask, *data); From 8c76d7cd520ebffc1ea9ea0850d87a224a50c7f2 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 6 Mar 2017 17:36:41 +0100 Subject: [PATCH 6/6] USB: serial: safe_serial: fix information leak in completion handler Add missing sanity check to the bulk-in completion handler to avoid an integer underflow that could be triggered by a malicious device. This avoids leaking up to 56 bytes from after the URB transfer buffer to user space. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/safe_serial.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/usb/serial/safe_serial.c b/drivers/usb/serial/safe_serial.c index 93c6c9b08daa..8a069aa154ed 100644 --- a/drivers/usb/serial/safe_serial.c +++ b/drivers/usb/serial/safe_serial.c @@ -200,6 +200,11 @@ static void safe_process_read_urb(struct urb *urb) if (!safe) goto out; + if (length < 2) { + dev_err(&port->dev, "malformed packet\n"); + return; + } + fcs = fcs_compute10(data, length, CRC10_INITFCS); if (fcs) { dev_err(&port->dev, "%s - bad CRC %x\n", __func__, fcs);