ALSA: usb-audio: Fix race against the error recovery URB submission (9b7e5208) · Commits · 戴 / test

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.de Signed-off-by:

Takashi Iwai <tiwai@suse.de>

sound/usb/midi.c

+12 −5

Original line number	Diff line number	Diff line
		@@ -1499,6 +1499,8 @@ void snd_usbmidi_disconnect(struct list_head *p)
		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)
		@@ -1525,7 +1527,6 @@ void snd_usbmidi_disconnect(struct list_head *p)
		ep->in = NULL;
		}
		}
		del_timer_sync(&umidi->error_timer);
		}
		EXPORT_SYMBOL(snd_usbmidi_disconnect);

		@@ -2301,16 +2302,22 @@ void snd_usbmidi_input_stop(struct list_head *p)
		}
		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];
		spin_lock_irqsave(&umidi->disc_lock, flags);
		if (!atomic_read(&urb->use_count)) {
		urb->dev = ep->umidi->dev;
		snd_usbmidi_submit_urb(urb, GFP_KERNEL);
		snd_usbmidi_submit_urb(urb, GFP_ATOMIC);
		}
		spin_unlock_irqrestore(&umidi->disc_lock, flags);
		}
		}

		@@ -2326,7 +2333,7 @@ void snd_usbmidi_input_start(struct list_head *p)
		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);

Admin message