BUG: KASAN: use-after-free in snd_complete_urb+0x109e/0x1740 [snd_usb_audio] (5.11-rc6)

[Hillf Danton]

On Sat, 6 Feb 2021 Takashi Iwai wrote:
> > Due to the reconnecting key word mentioned, no fix to
> > d0f09d1e4a88 ("ALSA: usb-audio: Refactoring endpoint URB deactivation")
> > will be added.
> > 
> > What is added is to capture EP_FLAG_STOPPING and remove the one
> > second wait limit if the reconnecting acts may make it easier to
> > repro the uaf. The diff is only for idea show.
> 
> If my understanding is right, this won't change.  The problem is
> rather the lack of this function call itself, i.e. the missing
> synchronization for the stream stop.

Thanks for taking a look at it.
> 
> It worked casually in the past because the endpoint resource is
> released at a later point that is after all streams are really closed.
> Now it's released earlier and hitting the UAF.

And add it if I dont misread you.

Hillf

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -832,24 +832,14 @@ void snd_usb_endpoint_suspend(struct snd
  */
 static int wait_clear_urbs(struct snd_usb_endpoint *ep)
 {
-	unsigned long end_time = jiffies + msecs_to_jiffies(1000);
-	int alive;
-
-	if (!test_bit(EP_FLAG_STOPPING, &ep->flags))
-		return 0;
-
+	WARN_ON_ONCE(!test_bit(EP_FLAG_STOPPING, &ep->flags));
 	do {
-		alive = bitmap_weight(&ep->active_mask, ep->nurbs);
-		if (!alive)
+		if (!bitmap_weight(&ep->active_mask, ep->nurbs))
 			break;
 
 		schedule_timeout_uninterruptible(1);
-	} while (time_before(jiffies, end_time));
+	} while (1);
 
-	if (alive)
-		usb_audio_err(ep->chip,
-			"timeout: still %d active urbs on EP #%x\n",
-			alive, ep->ep_num);
 	clear_bit(EP_FLAG_STOPPING, &ep->flags);
 
 	ep->sync_sink = NULL;
@@ -1433,7 +1423,7 @@ void snd_usb_endpoint_stop(struct snd_us
 		WRITE_ONCE(ep->sync_source->sync_sink, NULL);
 
 	if (!atomic_dec_return(&ep->running))
-		stop_and_unlink_urbs(ep, false, false);
+		stop_and_unlink_urbs(ep, false, true);
 }
 
 /**