Commit 86a42ad0 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: usb-audio: Fix possible race at sync of urb completions 

USB-audio driver tries to sync with the clear of all pending URBs in
wait_clear_urbs(), and it waits for all bits in active_mask getting
cleared.  This works fine for the normal operations, but when a stream
is managed in the implicit feedback mode, there is still a very thin
race window: namely, in snd_complete_usb(), the active_mask bit for
the current URB is once cleared before re-submitted in
queue_pending_output_urbs().  If wait_clear_urbs() is called during
that period, it may pass the test and go forward even though there may
be a still pending URB.

For covering it, this patch adds a new counter to each endpoint to
keep the number of in-flight URBs, and changes wait_clear_urbs()
checking this number instead.  The counter is decremented at the end
of URB complete, hence the reference is kept as long as the URB
complete is in process.

Link: https://lore.kernel.org/r/20210929080844.11583-3-tiwai@suse.de


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 4e7cf1fb

sound/usb/card.h

+1 −0
Original line number Diff line number Diff line
@@ -97,6 +97,7 @@ struct snd_usb_endpoint { 
	unsigned int nominal_queue_size; /* total buffer sizes in URBs */

	unsigned long active_mask;	/* bitmask of active urbs */

	unsigned long unlink_mask;	/* bitmask of unlinked urbs */

	atomic_t submitted_urbs;	/* currently submitted urbs */

	char *syncbuf;			/* sync buffer for all sync URBs */

	dma_addr_t sync_dma;		/* DMA address of syncbuf */

sound/usb/endpoint.c

+6 −1
Original line number Diff line number Diff line
@@ -451,6 +451,7 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) 
		}



		set_bit(ctx->index, &ep->active_mask);

		atomic_inc(&ep->submitted_urbs);

	}

}
@@ -488,6 +489,7 @@ static void snd_complete_urb(struct urb *urb) 
			clear_bit(ctx->index, &ep->active_mask);

			spin_unlock_irqrestore(&ep->lock, flags);

			queue_pending_output_urbs(ep);

			atomic_dec(&ep->submitted_urbs); /* decrement at last */

			return;

		}
@@ -513,6 +515,7 @@ static void snd_complete_urb(struct urb *urb) 


exit_clear:

	clear_bit(ctx->index, &ep->active_mask);

	atomic_dec(&ep->submitted_urbs);

}



/*
@@ -596,6 +599,7 @@ int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type) 
	ep->type = type;

	ep->ep_num = ep_num;

	INIT_LIST_HEAD(&ep->ready_playback_urbs);

	atomic_set(&ep->submitted_urbs, 0);



	is_playback = ((ep_num & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT);

	ep_num &= USB_ENDPOINT_NUMBER_MASK;
@@ -861,7 +865,7 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) 
		return 0;



	do {

		alive = bitmap_weight(&ep->active_mask, ep->nurbs);

		alive = atomic_read(&ep->submitted_urbs);

		if (!alive)

			break;
@@ -1441,6 +1445,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) 
			goto __error;

		}

		set_bit(i, &ep->active_mask);

		atomic_inc(&ep->submitted_urbs);

	}



	usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n",