On Wed, 25 Jun 2014, Takashi Iwai wrote: > [Adding Daniel to Cc] > > At Tue, 24 Jun 2014 13:52:14 -0400 (EDT), > Alan Stern wrote: > > > > Takashi and Clemens: > > > > The snd-usb-audio driver has a race between close and disconnect. A > > patch I have been testing reliably triggers this race on my machine and > > reveals a use-after-free bug. > > > > This happens when a device is disconnected while being used for audio > > I/O. Simply unplugging the device doesn't seem to trigger the problem, > > probably because it involves different timing. > > > > In detail: When the device is unplugged, the USB core calls > > usb_audio_disconnect() -> snd_usb_audio_disconnect(). > > > > At the same time, the user program gets a poll failure and closes the > > device: snd_usb_playback_close() -> snd_usb_pcm_close() -> > > stop_endpoints() -> snd_usb_endpoint_sync_pending_stop() -> > > wait_clear_urbs(). This routine waits until the endpoint's > > outstanding URBs have completed, testing the snd_usb_endpoint structure > > in a loop. > > > > Meanwhile, back in the disconnect thread, snd_usb_audio_disconnect() > > calls snd_usb_endpoint_free() for each endpoint on the device. This > > routine unlinks the endpoint's URBs, calls wait_clear_urbs(), and then > > deallocates the snd_usb_endpoint structure. > > > > So there are two threads both running wait_clear_urbs() for the same > > endpoint, and one of them will deallocate the endpoint afterward. If > > that thread happens to finish first, the other thread will dereference > > the deallocated structure. > > > > Obviously there needs to be some sort of mutual exclusion between the > > I/O pathways and the disconnect pathway. I don't know what the right > > solution is. The patch below at least avoids this particular failure > > scenario, but probably not correctly. > > > > Can either of you write a proper fix? > > If I understand correctly, calling wait_clear_urbs() simultaneously > doesn't do anything harmful. Also calling wait_clear_urbs() and > release_urbs() at the same time would be OK, too. The problem is > rather that snd_usb_endpoint_free() actually frees the ep object that > is being referenced. Moving kfree() into the later point should solve > it. > > Below is an untested patch. Give it a try. Thanks for the patch. I confirm that it fixed the bug. Tested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> Don't forget to tag it for -stable when you merge it! Alan Stern -- 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
