Skip to content

Commit

Permalink
USB: mos7840: fix race in led handling
Browse files Browse the repository at this point in the history
Fix race in LED handling introduced by commit 0eafe4d ("USB: serial:
mos7840: add support for MCS7810 devices") which reused the port control
urb for manipulating the LED without making sure that the urb is not
already in use. This could lead to the control urb being manipulated
while in flight.

Fix by adding a dedicated LED urb and ctrlrequest along with a LED-busy
flag to handle concurrency.

Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Johan Hovold authored and Greg Kroah-Hartman committed Jul 26, 2013
1 parent 40c24f2 commit 05cf0de
Showing 1 changed file with 37 additions and 22 deletions.
59 changes: 37 additions & 22 deletions drivers/usb/serial/mos7840.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@

enum mos7840_flag {
MOS7840_FLAG_CTRL_BUSY,
MOS7840_FLAG_LED_BUSY,
};

static const struct usb_device_id id_table[] = {
Expand Down Expand Up @@ -240,9 +241,10 @@ struct moschip_port {

/* For device(s) with LED indicator */
bool has_led;
bool led_flag;
struct timer_list led_timer1; /* Timer for LED on */
struct timer_list led_timer2; /* Timer for LED off */
struct urb *led_urb;
struct usb_ctrlrequest *led_dr;

unsigned long flags;
};
Expand Down Expand Up @@ -535,18 +537,18 @@ static void mos7840_set_led_async(struct moschip_port *mcs, __u16 wval,
__u16 reg)
{
struct usb_device *dev = mcs->port->serial->dev;
struct usb_ctrlrequest *dr = mcs->dr;
struct usb_ctrlrequest *dr = mcs->led_dr;

dr->bRequestType = MCS_WR_RTYPE;
dr->bRequest = MCS_WRREQ;
dr->wValue = cpu_to_le16(wval);
dr->wIndex = cpu_to_le16(reg);
dr->wLength = cpu_to_le16(0);

usb_fill_control_urb(mcs->control_urb, dev, usb_sndctrlpipe(dev, 0),
usb_fill_control_urb(mcs->led_urb, dev, usb_sndctrlpipe(dev, 0),
(unsigned char *)dr, NULL, 0, mos7840_set_led_callback, NULL);

usb_submit_urb(mcs->control_urb, GFP_ATOMIC);
usb_submit_urb(mcs->led_urb, GFP_ATOMIC);
}

static void mos7840_set_led_sync(struct usb_serial_port *port, __u16 reg,
Expand All @@ -572,7 +574,19 @@ static void mos7840_led_flag_off(unsigned long arg)
{
struct moschip_port *mcs = (struct moschip_port *) arg;

mcs->led_flag = false;
clear_bit_unlock(MOS7840_FLAG_LED_BUSY, &mcs->flags);
}

static void mos7840_led_activity(struct usb_serial_port *port)
{
struct moschip_port *mos7840_port = usb_get_serial_port_data(port);

if (test_and_set_bit_lock(MOS7840_FLAG_LED_BUSY, &mos7840_port->flags))
return;

mos7840_set_led_async(mos7840_port, 0x0301, MODEM_CONTROL_REGISTER);
mod_timer(&mos7840_port->led_timer1,
jiffies + msecs_to_jiffies(LED_ON_MS));
}

/*****************************************************************************
Expand Down Expand Up @@ -770,14 +784,8 @@ static void mos7840_bulk_in_callback(struct urb *urb)
return;
}

/* Turn on LED */
if (mos7840_port->has_led && !mos7840_port->led_flag) {
mos7840_port->led_flag = true;
mos7840_set_led_async(mos7840_port, 0x0301,
MODEM_CONTROL_REGISTER);
mod_timer(&mos7840_port->led_timer1,
jiffies + msecs_to_jiffies(LED_ON_MS));
}
if (mos7840_port->has_led)
mos7840_led_activity(port);

mos7840_port->read_urb_busy = true;
retval = usb_submit_urb(mos7840_port->read_urb, GFP_ATOMIC);
Expand Down Expand Up @@ -1454,13 +1462,8 @@ static int mos7840_write(struct tty_struct *tty, struct usb_serial_port *port,
data1 = urb->transfer_buffer;
dev_dbg(&port->dev, "bulkout endpoint is %d\n", port->bulk_out_endpointAddress);

/* Turn on LED */
if (mos7840_port->has_led && !mos7840_port->led_flag) {
mos7840_port->led_flag = true;
mos7840_set_led_sync(port, MODEM_CONTROL_REGISTER, 0x0301);
mod_timer(&mos7840_port->led_timer1,
jiffies + msecs_to_jiffies(LED_ON_MS));
}
if (mos7840_port->has_led)
mos7840_led_activity(port);

/* send it down the pipe */
status = usb_submit_urb(urb, GFP_ATOMIC);
Expand Down Expand Up @@ -2412,6 +2415,14 @@ static int mos7840_port_probe(struct usb_serial_port *port)
if (device_type == MOSCHIP_DEVICE_ID_7810) {
mos7840_port->has_led = true;

mos7840_port->led_urb = usb_alloc_urb(0, GFP_KERNEL);
mos7840_port->led_dr = kmalloc(sizeof(*mos7840_port->led_dr),
GFP_KERNEL);
if (!mos7840_port->led_urb || !mos7840_port->led_dr) {
status = -ENOMEM;
goto error;
}

init_timer(&mos7840_port->led_timer1);
mos7840_port->led_timer1.function = mos7840_led_off;
mos7840_port->led_timer1.expires =
Expand All @@ -2424,8 +2435,6 @@ static int mos7840_port_probe(struct usb_serial_port *port)
jiffies + msecs_to_jiffies(LED_OFF_MS);
mos7840_port->led_timer2.data = (unsigned long)mos7840_port;

mos7840_port->led_flag = false;

/* Turn off LED */
mos7840_set_led_sync(port, MODEM_CONTROL_REGISTER, 0x0300);
}
Expand All @@ -2447,6 +2456,8 @@ static int mos7840_port_probe(struct usb_serial_port *port)
}
return 0;
error:
kfree(mos7840_port->led_dr);
usb_free_urb(mos7840_port->led_urb);
kfree(mos7840_port->dr);
kfree(mos7840_port->ctrl_buf);
usb_free_urb(mos7840_port->control_urb);
Expand All @@ -2467,6 +2478,10 @@ static int mos7840_port_remove(struct usb_serial_port *port)

del_timer_sync(&mos7840_port->led_timer1);
del_timer_sync(&mos7840_port->led_timer2);

usb_kill_urb(mos7840_port->led_urb);
usb_free_urb(mos7840_port->led_urb);
kfree(mos7840_port->led_dr);
}
usb_kill_urb(mos7840_port->control_urb);
usb_free_urb(mos7840_port->control_urb);
Expand Down

0 comments on commit 05cf0de

Please sign in to comment.