[PATCH 4.14 096/125] ALSA: usb-audio: Fix race against the error recovery URB submission

From: Greg Kroah-Hartman
Date: Mon Jul 20 2020 - 11:47:57 EST


From: Takashi Iwai <tiwai@xxxxxxx>

commit 9b7e5208a941e2e491a83eb5fa83d889e888fa2f 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@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+5f1d24c49c1d2c427497@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: <stable@xxxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20200710160656.16819-1-tiwai@xxxxxxx
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
sound/usb/midi.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1477,6 +1477,8 @@ void snd_usbmidi_disconnect(struct list_
spin_unlock_irq(&umidi->disc_lock);
up_write(&umidi->disc_rwsem);

+ del_timer_sync(&umidi->error_timer);
+
for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i];
if (ep->out)
@@ -1503,7 +1505,6 @@ void snd_usbmidi_disconnect(struct list_
ep->in = NULL;
}
}
- del_timer_sync(&umidi->error_timer);
}
EXPORT_SYMBOL(snd_usbmidi_disconnect);

@@ -2260,16 +2261,22 @@ void snd_usbmidi_input_stop(struct list_
}
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 long flags;

if (!ep)
return;
for (i = 0; i < INPUT_URBS; ++i) {
struct urb *urb = ep->urbs[i];
- urb->dev = ep->umidi->dev;
- snd_usbmidi_submit_urb(urb, GFP_KERNEL);
+ spin_lock_irqsave(&umidi->disc_lock, flags);
+ 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);
}
}

@@ -2285,7 +2292,7 @@ void snd_usbmidi_input_start(struct list
if (umidi->input_running || !umidi->opened[1])
return;
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;
}
EXPORT_SYMBOL(snd_usbmidi_input_start);