Developer Forum

by **Oliver Neu** » Wed, 24 Oct 2007 18:40:09

m Dienstag 23 Oktober 2007 schrieb Pete Zaitcev:

Why bother? Simply call usb_kill_urb() unconditionally.

This leaves a race here:

while (count > 0) {
spin_lock_irqsave(&dev->buflock, flags);
if (!dev->out_urb_finished) {
spin_unlock_irqrestore(&dev->buflock, flags);

timeout = COMMAND_TIMEOUT;
while (timeout > 0) {
if (signal_pending(current)) {
dbg(1," %s : interrupted", __FUNCTION__);
retval = -EINTR;
goto exit;
}
mutex_unlock(&dev->mtx);
timeout = interruptible_sleep_on_timeout(&dev->write_wait, timeout);

You can detect that the urb has not finished but the notification happens before
you go to sleep.

This is racy:
dev = usb_get_intfdata(interface);
if (!dev) {
retval = -ENODEV;
goto exit_no_device;
}

if ((retval = mutex_lock_interruptible(&adutux_mutex))) {
dbg(2, "%s : mutex lock failed", __FUNCTION__);
goto exit_no_device;
}

You need to manipulate intfdata under lock, or disconnect will
happily free the datastructures.

You should set file->private_data to NULL in the error case.

[..]

I find it a bit silly to bother with _irqsave if you call schedule_timeout() in the
next line.

Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Developer Forum

[linux-usb-devel] USB: FIx locks and urb->status in adutux

[linux-usb-devel] USB: FIx locks and urb->status in adutux