Oliver Neukum <oliver@xxxxxxxxxx> writes:
> Am Montag, 2. Juli 2012, 01:08:59 schrieb Bjørn Mork:
>> The WDM_READ flag is normally cleared by wdm_int_callback
>> before resubmitting the interrupt urb. But a crashing
>> device may cause both a read error and cancelling all
>> urbs. We must make sure that the flag is cleared by
>> wdm_read in this case.
>>
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
>> ---
>> drivers/usb/class/cdc-wdm.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
>> index 25e7d72..f77a631 100644
>> --- a/drivers/usb/class/cdc-wdm.c
>> +++ b/drivers/usb/class/cdc-wdm.c
>> @@ -461,6 +461,8 @@ retry:
>> spin_lock_irq(&desc->iuspin);
>>
>> if (desc->rerr) { /* read completed, error happened */
>> + dev_dbg(&desc->intf->dev, "%s: error - clearing WDM_READ\n", __func__);
>> + clear_bit(WDM_READ, &desc->flags);
>> desc->rerr = 0;
>> spin_unlock_irq(&desc->iuspin);
>> rv = -EIO;
>> @@ -475,6 +477,8 @@ retry:
>> goto retry;
>> }
>> if (!desc->reslength) { /* zero length read */
>> + dev_dbg(&desc->intf->dev, "%s: zero length - clearing WDM_READ\n", __func__);
>> + clear_bit(WDM_READ, &desc->flags);
>> spin_unlock_irq(&desc->iuspin);
>> goto retry;
>> }
>
> Are you sure that both patches are needed? After the first error is reported
> we will return to user space and run into the reslength check at the next
> call.
Yes, we'll just do an extra wdm_read and be fine it can be dropped for
the error case.
> If we cleared the flag unconditionally on reporting an error it seems
> to me that we are introducing a data leak if a second read after the one
> that produced the error has already completed.
I guess you are right. I do have some problems following the logic of
this flag... But to my defense I will claim that so did the original
author ;-)
We could create an additonal !desc->reslength condition for the error
case, but as you point out we'll only loose one round through the loop
if we don't. So better just drop it. I'll send an updated patch.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
