* [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check
[not found] <20200122101530.29176-1-johan@kernel.org>
@ 2020-01-22 10:15 ` Johan Hovold
2020-01-22 13:25 ` Greg KH
2020-01-22 10:15 ` [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling Johan Hovold
2020-01-22 10:15 ` [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing Johan Hovold
2 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2020-01-22 10:15 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable
Add missing endpoint sanity check to avoid dereferencing a NULL-pointer
on open() in case a device lacks a bulk-out endpoint.
Note that prior to commit f4a4cbb2047e ("USB: ir-usb: reimplement using
generic framework") the oops would instead happen on open() if the
device lacked a bulk-in endpoint and on write() if it lacked a bulk-out
endpoint.
Fixes: f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/ir-usb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/serial/ir-usb.c b/drivers/usb/serial/ir-usb.c
index 302eb9530859..c3b06fc5a7f0 100644
--- a/drivers/usb/serial/ir-usb.c
+++ b/drivers/usb/serial/ir-usb.c
@@ -195,6 +195,9 @@ static int ir_startup(struct usb_serial *serial)
struct usb_irda_cs_descriptor *irda_desc;
int rates;
+ if (serial->num_bulk_in < 1 || serial->num_bulk_out < 1)
+ return -ENODEV;
+
irda_desc = irda_usb_find_class_desc(serial, 0);
if (!irda_desc) {
dev_err(&serial->dev->dev,
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling
[not found] <20200122101530.29176-1-johan@kernel.org>
2020-01-22 10:15 ` [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check Johan Hovold
@ 2020-01-22 10:15 ` Johan Hovold
2020-01-22 13:24 ` Greg KH
2020-01-22 10:15 ` [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing Johan Hovold
2 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2020-01-22 10:15 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable, Felipe Balbi
Commit e0d795e4f36c ("usb: irda: cleanup on ir-usb module") added a USB
IrDA header with common defines, but mistakingly switched to using the
class-descriptor baud-rate bitmask values for the outbound header.
This broke link-speed handling for rates above 9600 baud, but a device
would also be able to operate at the default 9600 baud until a
link-speed request was issued (e.g. using the TCGETS ioctl).
Fixes: e0d795e4f36c ("usb: irda: cleanup on ir-usb module")
Cc: stable <stable@vger.kernel.org> # 2.6.27
Cc: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/ir-usb.c | 20 ++++++++++----------
include/linux/usb/irda.h | 13 ++++++++++++-
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/serial/ir-usb.c b/drivers/usb/serial/ir-usb.c
index c3b06fc5a7f0..26eab1307165 100644
--- a/drivers/usb/serial/ir-usb.c
+++ b/drivers/usb/serial/ir-usb.c
@@ -335,34 +335,34 @@ static void ir_set_termios(struct tty_struct *tty,
switch (baud) {
case 2400:
- ir_baud = USB_IRDA_BR_2400;
+ ir_baud = USB_IRDA_LS_2400;
break;
case 9600:
- ir_baud = USB_IRDA_BR_9600;
+ ir_baud = USB_IRDA_LS_9600;
break;
case 19200:
- ir_baud = USB_IRDA_BR_19200;
+ ir_baud = USB_IRDA_LS_19200;
break;
case 38400:
- ir_baud = USB_IRDA_BR_38400;
+ ir_baud = USB_IRDA_LS_38400;
break;
case 57600:
- ir_baud = USB_IRDA_BR_57600;
+ ir_baud = USB_IRDA_LS_57600;
break;
case 115200:
- ir_baud = USB_IRDA_BR_115200;
+ ir_baud = USB_IRDA_LS_115200;
break;
case 576000:
- ir_baud = USB_IRDA_BR_576000;
+ ir_baud = USB_IRDA_LS_576000;
break;
case 1152000:
- ir_baud = USB_IRDA_BR_1152000;
+ ir_baud = USB_IRDA_LS_1152000;
break;
case 4000000:
- ir_baud = USB_IRDA_BR_4000000;
+ ir_baud = USB_IRDA_LS_4000000;
break;
default:
- ir_baud = USB_IRDA_BR_9600;
+ ir_baud = USB_IRDA_LS_9600;
baud = 9600;
}
diff --git a/include/linux/usb/irda.h b/include/linux/usb/irda.h
index 396d2b043e64..556a801efce3 100644
--- a/include/linux/usb/irda.h
+++ b/include/linux/usb/irda.h
@@ -119,11 +119,22 @@ struct usb_irda_cs_descriptor {
* 6 - 115200 bps
* 7 - 576000 bps
* 8 - 1.152 Mbps
- * 9 - 5 mbps
+ * 9 - 4 Mbps
* 10..15 - Reserved
*/
#define USB_IRDA_STATUS_LINK_SPEED 0x0f
+#define USB_IRDA_LS_NO_CHANGE 0
+#define USB_IRDA_LS_2400 1
+#define USB_IRDA_LS_9600 2
+#define USB_IRDA_LS_19200 3
+#define USB_IRDA_LS_38400 4
+#define USB_IRDA_LS_57600 5
+#define USB_IRDA_LS_115200 6
+#define USB_IRDA_LS_576000 7
+#define USB_IRDA_LS_1152000 8
+#define USB_IRDA_LS_4000000 9
+
/* The following is a 4-bit value used only for
* outbound header:
*
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing
[not found] <20200122101530.29176-1-johan@kernel.org>
2020-01-22 10:15 ` [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check Johan Hovold
2020-01-22 10:15 ` [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling Johan Hovold
@ 2020-01-22 10:15 ` Johan Hovold
2020-01-22 13:25 ` Greg KH
2 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2020-01-22 10:15 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable
Commit f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
switched to using the generic write implementation which may combine
multiple write requests into larger transfers. This can break the IrLAP
protocol where end-of-frame is determined using the USB short packet
mechanism, for example, if multiple frames are sent in rapid succession.
Fixes: f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
Cc: stable <stable@vger.kernel.org> # 2.6.35
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/ir-usb.c | 113 +++++++++++++++++++++++++++++-------
1 file changed, 91 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/ir-usb.c b/drivers/usb/serial/ir-usb.c
index 26eab1307165..627bea7e6cfb 100644
--- a/drivers/usb/serial/ir-usb.c
+++ b/drivers/usb/serial/ir-usb.c
@@ -45,9 +45,10 @@ static int buffer_size;
static int xbof = -1;
static int ir_startup (struct usb_serial *serial);
-static int ir_open(struct tty_struct *tty, struct usb_serial_port *port);
-static int ir_prepare_write_buffer(struct usb_serial_port *port,
- void *dest, size_t size);
+static int ir_write(struct tty_struct *tty, struct usb_serial_port *port,
+ const unsigned char *buf, int count);
+static int ir_write_room(struct tty_struct *tty);
+static void ir_write_bulk_callback(struct urb *urb);
static void ir_process_read_urb(struct urb *urb);
static void ir_set_termios(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios);
@@ -77,8 +78,9 @@ static struct usb_serial_driver ir_device = {
.num_ports = 1,
.set_termios = ir_set_termios,
.attach = ir_startup,
- .open = ir_open,
- .prepare_write_buffer = ir_prepare_write_buffer,
+ .write = ir_write,
+ .write_room = ir_write_room,
+ .write_bulk_callback = ir_write_bulk_callback,
.process_read_urb = ir_process_read_urb,
};
@@ -254,35 +256,102 @@ static int ir_startup(struct usb_serial *serial)
return 0;
}
-static int ir_open(struct tty_struct *tty, struct usb_serial_port *port)
+static int ir_write(struct tty_struct *tty, struct usb_serial_port *port,
+ const unsigned char *buf, int count)
{
- int i;
+ struct urb *urb = NULL;
+ unsigned long flags;
+ int ret;
- for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
- port->write_urbs[i]->transfer_flags = URB_ZERO_PACKET;
+ if (port->bulk_out_size == 0)
+ return -EINVAL;
- /* Start reading from the device */
- return usb_serial_generic_open(tty, port);
-}
+ if (count == 0)
+ return 0;
-static int ir_prepare_write_buffer(struct usb_serial_port *port,
- void *dest, size_t size)
-{
- unsigned char *buf = dest;
- int count;
+ count = min(count, port->bulk_out_size - 1);
+
+ spin_lock_irqsave(&port->lock, flags);
+ if (__test_and_clear_bit(0, &port->write_urbs_free)) {
+ urb = port->write_urbs[0];
+ port->tx_bytes += count;
+ }
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (!urb)
+ return 0;
/*
* The first byte of the packet we send to the device contains an
- * inbound header which indicates an additional number of BOFs and
+ * outbound header which indicates an additional number of BOFs and
* a baud rate change.
*
* See section 5.4.2.2 of the USB IrDA spec.
*/
- *buf = ir_xbof | ir_baud;
+ *(u8 *)urb->transfer_buffer = ir_xbof | ir_baud;
+
+ memcpy(urb->transfer_buffer + 1, buf, count);
+
+ urb->transfer_buffer_length = count + 1;
+ urb->transfer_flags = URB_ZERO_PACKET;
+
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ if (ret) {
+ dev_err(&port->dev, "failed to submit write urb: %d\n", ret);
+
+ spin_lock_irqsave(&port->lock, flags);
+ __set_bit(0, &port->write_urbs_free);
+ port->tx_bytes -= count;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ return ret;
+ }
+
+ return count;
+}
+
+static void ir_write_bulk_callback(struct urb *urb)
+{
+ struct usb_serial_port *port = urb->context;
+ int status = urb->status;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ __set_bit(0, &port->write_urbs_free);
+ port->tx_bytes -= urb->transfer_buffer_length - 1;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ switch (status) {
+ case 0:
+ break;
+ case -ENOENT:
+ case -ECONNRESET:
+ case -ESHUTDOWN:
+ dev_dbg(&port->dev, "write urb stopped: %d\n", status);
+ return;
+ case -EPIPE:
+ dev_err(&port->dev, "write urb stopped: %d\n", status);
+ return;
+ default:
+ dev_err(&port->dev, "nonzero write-urb status: %d\n", status);
+ break;
+ }
+
+ usb_serial_port_softint(port);
+}
+
+static int ir_write_room(struct tty_struct *tty)
+{
+ struct usb_serial_port *port = tty->driver_data;
+ int count = 0;
+
+ if (port->bulk_out_size == 0)
+ return 0;
+
+ if (test_bit(0, &port->write_urbs_free))
+ count = port->bulk_out_size - 1;
- count = kfifo_out_locked(&port->write_fifo, buf + 1, size - 1,
- &port->lock);
- return count + 1;
+ return count;
}
static void ir_process_read_urb(struct urb *urb)
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling
2020-01-22 10:15 ` [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling Johan Hovold
@ 2020-01-22 13:24 ` Greg KH
2020-01-22 13:35 ` Johan Hovold
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2020-01-22 13:24 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable, Felipe Balbi
On Wed, Jan 22, 2020 at 11:15:27AM +0100, Johan Hovold wrote:
> Commit e0d795e4f36c ("usb: irda: cleanup on ir-usb module") added a USB
> IrDA header with common defines, but mistakingly switched to using the
> class-descriptor baud-rate bitmask values for the outbound header.
>
> This broke link-speed handling for rates above 9600 baud, but a device
> would also be able to operate at the default 9600 baud until a
> link-speed request was issued (e.g. using the TCGETS ioctl).
People still use this driver/hardware? And this wasn't found until now?
wow...
> Fixes: e0d795e4f36c ("usb: irda: cleanup on ir-usb module")
> Cc: stable <stable@vger.kernel.org> # 2.6.27
> Cc: Felipe Balbi <balbi@kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check
2020-01-22 10:15 ` [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check Johan Hovold
@ 2020-01-22 13:25 ` Greg KH
0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2020-01-22 13:25 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable
On Wed, Jan 22, 2020 at 11:15:26AM +0100, Johan Hovold wrote:
> Add missing endpoint sanity check to avoid dereferencing a NULL-pointer
> on open() in case a device lacks a bulk-out endpoint.
>
> Note that prior to commit f4a4cbb2047e ("USB: ir-usb: reimplement using
> generic framework") the oops would instead happen on open() if the
> device lacked a bulk-in endpoint and on write() if it lacked a bulk-out
> endpoint.
>
> Fixes: f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing
2020-01-22 10:15 ` [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing Johan Hovold
@ 2020-01-22 13:25 ` Greg KH
2020-01-22 13:37 ` Johan Hovold
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2020-01-22 13:25 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, stable
On Wed, Jan 22, 2020 at 11:15:28AM +0100, Johan Hovold wrote:
> Commit f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
> switched to using the generic write implementation which may combine
> multiple write requests into larger transfers. This can break the IrLAP
> protocol where end-of-frame is determined using the USB short packet
> mechanism, for example, if multiple frames are sent in rapid succession.
>
> Fixes: f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
> Cc: stable <stable@vger.kernel.org> # 2.6.35
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/usb/serial/ir-usb.c | 113 +++++++++++++++++++++++++++++-------
> 1 file changed, 91 insertions(+), 22 deletions(-)
Ah, nice fix, sorry about that :(
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling
2020-01-22 13:24 ` Greg KH
@ 2020-01-22 13:35 ` Johan Hovold
0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2020-01-22 13:35 UTC (permalink / raw)
To: Greg KH; +Cc: Johan Hovold, linux-usb, stable, Felipe Balbi
On Wed, Jan 22, 2020 at 02:24:50PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 22, 2020 at 11:15:27AM +0100, Johan Hovold wrote:
> > Commit e0d795e4f36c ("usb: irda: cleanup on ir-usb module") added a USB
> > IrDA header with common defines, but mistakingly switched to using the
> > class-descriptor baud-rate bitmask values for the outbound header.
> >
> > This broke link-speed handling for rates above 9600 baud, but a device
> > would also be able to operate at the default 9600 baud until a
> > link-speed request was issued (e.g. using the TCGETS ioctl).
>
> People still use this driver/hardware? And this wasn't found until now?
> wow...
Good question. I was considering just removing the driver, but since it
would have continued to work on low and default link speed I figured I
better just fix it up.
> > Fixes: e0d795e4f36c ("usb: irda: cleanup on ir-usb module")
> > Cc: stable <stable@vger.kernel.org> # 2.6.27
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
>
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks for reviewing.
Johan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing
2020-01-22 13:25 ` Greg KH
@ 2020-01-22 13:37 ` Johan Hovold
0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2020-01-22 13:37 UTC (permalink / raw)
To: Greg KH; +Cc: Johan Hovold, linux-usb, stable
On Wed, Jan 22, 2020 at 02:25:42PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 22, 2020 at 11:15:28AM +0100, Johan Hovold wrote:
> > Commit f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
> > switched to using the generic write implementation which may combine
> > multiple write requests into larger transfers. This can break the IrLAP
> > protocol where end-of-frame is determined using the USB short packet
> > mechanism, for example, if multiple frames are sent in rapid succession.
> >
> > Fixes: f4a4cbb2047e ("USB: ir-usb: reimplement using generic framework")
> > Cc: stable <stable@vger.kernel.org> # 2.6.35
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> > drivers/usb/serial/ir-usb.c | 113 +++++++++++++++++++++++++++++-------
> > 1 file changed, 91 insertions(+), 22 deletions(-)
>
> Ah, nice fix, sorry about that :(
The offending commit was mine so same to you. ;)
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Johan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-22 13:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200122101530.29176-1-johan@kernel.org>
2020-01-22 10:15 ` [PATCH 1/5] USB: serial: ir-usb: add missing endpoint sanity check Johan Hovold
2020-01-22 13:25 ` Greg KH
2020-01-22 10:15 ` [PATCH 2/5] USB: serial: ir-usb: fix link-speed handling Johan Hovold
2020-01-22 13:24 ` Greg KH
2020-01-22 13:35 ` Johan Hovold
2020-01-22 10:15 ` [PATCH 3/5] USB: serial: ir-usb: fix IrLAP framing Johan Hovold
2020-01-22 13:25 ` Greg KH
2020-01-22 13:37 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).