Re: [PATCH] cdc-wdm: fix race between interrupt handler and tasklet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



oliver@xxxxxxxxxx writes:

> From: Oliver Neukum <oneukum@xxxxxxx>
>
> Both could want to submit the same URB. Some checks of the flag
> intended to prevent that were missing.
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxx>
> CC: stable@xxxxxxxxxxxxxxx

Yes, this is definitely required for 3.11 and stable.  Note that it is
not just fixing a race between the interrupt handler and the tasklet,
but also the case where another RESPONSE_AVAILABLE notification is
received before the last read is finished.

There are devices sending multiple RESPONSE_AVAILABLE notifications
back-to-back when the reponse data exceeds wMaxCommand, without waiting
for the driver to finish reading the first part.  This will cause such
collisions as reported here:
http://ubuntuforums.org/showthread.php?t=2165362&p=12762195#post12762195

> @@ -262,8 +263,8 @@ static void wdm_int_callback(struct urb *urb)
>  
>  	spin_lock(&desc->iuspin);
>  	clear_bit(WDM_READ, &desc->flags);
> -	set_bit(WDM_RESPONDING, &desc->flags);
> -	if (!test_bit(WDM_DISCONNECTING, &desc->flags)
> +	responding = test_and_set_bit(WDM_RESPONDING, &desc->flags);
> +	if (!responding && !test_bit(WDM_DISCONNECTING, &desc->flags)
>  		&& !test_bit(WDM_SUSPENDING, &desc->flags)) {
>  		rv = usb_submit_urb(desc->response, GFP_ATOMIC);
>  		dev_dbg(&desc->intf->dev, "%s: usb_submit_urb %d",


I believe this still needs to be improved for 3.12.  We are currently
dropping RESPONSE_AVAILABLE notifications on the floor here if the
response URB is active.  This will cause the driver to miss responses,
in particular multi-part responses from devices sending the
notifications back-to-back.

Greg Suarez had some RFC patches a few moths ago, using a counter to
ensure a one-to-one relationship between received RESPONSE_AVAILABLE
notifications and the responses read. I've sent him an email asking
about the current status of that work.  Something like that is
necessary. 


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]