Commit 21203e84 authored by Takashi Iwai's avatar Takashi Iwai Committed by Khalid Elmously

ALSA: usb-audio: Fix race against the error recovery URB submission

BugLink: https://bugs.launchpad.net/bugs/1888690

commit 9b7e5208 upstream.

USB MIDI driver has an error recovery mechanism to resubmit the URB in
the delayed timer handler, and this may race with the standard start /
stop operations.  Although both start and stop operations themselves
don't race with each other due to the umidi->mutex protection, but
this isn't applied to the timer handler.

For fixing this potential race, the following changes are applied:

- Since the timer handler can't use the mutex, we apply the
  umidi->disc_lock protection at each input stream URB submission;
  this also needs to change the GFP flag to GFP_ATOMIC
- Add a check of the URB refcount and skip if already submitted
- Move the timer cancel call at disconnection to the beginning of the
  procedure; this assures the in-flight timer handler is gone properly
  before killing all pending URBs

Reported-by: syzbot+0f4ecfe6a2c322c81728@syzkaller.appspotmail.com
Reported-by: syzbot+5f1d24c49c1d2c427497@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20200710160656.16819-1-tiwai@suse.deSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarKamal Mostafa <kamal@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent efaac8f6
...@@ -1475,6 +1475,8 @@ void snd_usbmidi_disconnect(struct list_head *p) ...@@ -1475,6 +1475,8 @@ void snd_usbmidi_disconnect(struct list_head *p)
spin_unlock_irq(&umidi->disc_lock); spin_unlock_irq(&umidi->disc_lock);
up_write(&umidi->disc_rwsem); up_write(&umidi->disc_rwsem);
del_timer_sync(&umidi->error_timer);
for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) { for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i]; struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i];
if (ep->out) if (ep->out)
...@@ -1501,7 +1503,6 @@ void snd_usbmidi_disconnect(struct list_head *p) ...@@ -1501,7 +1503,6 @@ void snd_usbmidi_disconnect(struct list_head *p)
ep->in = NULL; ep->in = NULL;
} }
} }
del_timer_sync(&umidi->error_timer);
} }
EXPORT_SYMBOL(snd_usbmidi_disconnect); EXPORT_SYMBOL(snd_usbmidi_disconnect);
...@@ -2258,16 +2259,22 @@ void snd_usbmidi_input_stop(struct list_head *p) ...@@ -2258,16 +2259,22 @@ void snd_usbmidi_input_stop(struct list_head *p)
} }
EXPORT_SYMBOL(snd_usbmidi_input_stop); EXPORT_SYMBOL(snd_usbmidi_input_stop);
static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint *ep) static void snd_usbmidi_input_start_ep(struct snd_usb_midi *umidi,
struct snd_usb_midi_in_endpoint *ep)
{ {
unsigned int i; unsigned int i;
unsigned long flags;
if (!ep) if (!ep)
return; return;
for (i = 0; i < INPUT_URBS; ++i) { for (i = 0; i < INPUT_URBS; ++i) {
struct urb *urb = ep->urbs[i]; struct urb *urb = ep->urbs[i];
urb->dev = ep->umidi->dev; spin_lock_irqsave(&umidi->disc_lock, flags);
snd_usbmidi_submit_urb(urb, GFP_KERNEL); if (!atomic_read(&urb->use_count)) {
urb->dev = ep->umidi->dev;
snd_usbmidi_submit_urb(urb, GFP_ATOMIC);
}
spin_unlock_irqrestore(&umidi->disc_lock, flags);
} }
} }
...@@ -2283,7 +2290,7 @@ void snd_usbmidi_input_start(struct list_head *p) ...@@ -2283,7 +2290,7 @@ void snd_usbmidi_input_start(struct list_head *p)
if (umidi->input_running || !umidi->opened[1]) if (umidi->input_running || !umidi->opened[1])
return; return;
for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i)
snd_usbmidi_input_start_ep(umidi->endpoints[i].in); snd_usbmidi_input_start_ep(umidi, umidi->endpoints[i].in);
umidi->input_running = 1; umidi->input_running = 1;
} }
EXPORT_SYMBOL(snd_usbmidi_input_start); EXPORT_SYMBOL(snd_usbmidi_input_start);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment