On Thu, 11 Oct 2012, Matthieu CASTET wrote:

> Hi,
> 
> while doing some monkey tests on a product we found races in usb audio code 
> when
> the device in unplugged from usb (on linus master tree).
> 
> This can be reproduced with usb_audio_show_race.diff and CONFIG_DEBUG_SLAB.
> With this patch, start a stream :
> # arecord -D hw:0 -r 44100 -c 2  -f S16_LE > /dev/null
> you will see the kernel log : "in snd_usb_hw_params sleeping"
> Unplug the device before "in snd_usb_hw_params sleeping exit", and you will 
> see
> an oops in snd_pcm_hw_params
> 
> 
> Instead of using CONFIG_DEBUG_SLAB, usb_audio_show_use_after_free.diff can 
> show
> use after free by setting usb_device pointer to NULL in
> snd_usb_audio_disconnect. [1]
> 
> 
> In order to protect from all the races, before using any usb_device we need to
> check if it is not freed.

This should never be a problem.  Take a reference to the interface 
(usb_get_intf) and drop the reference (usb_put_intf) when you are 
finished accessing the usb_device structure.  This will prevent the 
usb_device from being deallocated while it is in use.

More important is a restriction you did not mention: Drivers are not 
allowed to communicate with a USB device after their disconnect routine 
has returned.

> What's the best way to do that ?

You could protect all occurrences of usb_submit_urb() with a spinlock.  
While holding the lock, check a flag to see if the disconnect routine
has been called.

In the disconnect routine, set this flag while holding the spinlock and 
then kill all the outstanding URBs.

I'm not very familiar with the snd-usb-audio driver; maybe it already 
does some of these things.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to