On Sat, 27 Sep 2003 18:35:22 +0200
Oliver Neukum <[EMAIL PROTECTED]> wrote:
> Am Samstag, 27. September 2003 14:56 schrieb Luca Risolia:
>
> Hi,
>
> congratulations to the driver, it is looking good.
> However, I've found one somewhat obscure bug.
> This piece of code has a small race condition in the error case
> on SMP.
> If a submission fails after another URB has been transmitted
> successfully, such an URB may be just executing its completion
> handler. In this case usb_unlink_urb will fail and already freed
> memory may be accessed.
>
> > + /* Submit the URBs */
> > + for (i = 0; i < W9968CF_URBS; i++) {
> > + err = usb_submit_urb(cam->urb[i]);
> > + if (err) {
> > + for (j = i-1; j >= 0; j--)
> > + usb_unlink_urb(cam->urb[j]);
> > + DBG(1, "Couldn't send a transfer request to the "
> > + "USB core (error #%d, %s).", err,
> > + symbolic(urb_errlist, err))
> > + goto free_urbs;
> > + }
> > + }
> > +
Yes, it is a bug. I've found it in other drivers too, when stopping the
isoc transfer, although they work "almost" correctly when an urb is submitted
and the next submission fails: in this case urbs are never freed.
Given that that solution seems to be accepted, I can fix the bug the same
way, by freeing the urbs "in the best case":
/* Submit the URBs */
for (i = 0; i < W9968CF_URBS; i++) {
err = usb_submit_urb(cam->urb[i]);
if (err) {
for (j = i-1; j >= 0; j--)
if (!usb_unlink_urb(cam->urb[j]))
usb_free_urb(cam->urb[j])
DBG(1, "Couldn't send a transfer request to the "
"USB core (error #%d, %s).", err,
symbolic(urb_errlist, err))
return err;
}
}
The same in stop_transfer():
/* This avoids race conditions with usb_submit_urb()
in the URB completition handler */
spin_lock_irqsave(&cam->urb_lock, lock_flags);
cam->streaming = 0;
spin_unlock_irqrestore(&cam->urb_lock, lock_flags);
for (i = W9968CF_URBS-1; i >= 0; i--)
if (cam->urb[i]) {
if (!usb_unlink_urb(cam->urb[i])) {
usb_free_urb(cam->urb[i]);
cam->urb[i] = NULL;
}
}
Thanks for the help.
Luca
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel