Patchwork ALSA: usb-audio: Fix regression by disconnection-race-fix patch

login
register
mail settings
Submitter Chris J Arges
Date Jan. 30, 2013, 3:22 p.m.
Message ID <1359559329-6021-1-git-send-email-chris.j.arges@canonical.com>
Download mbox | patch
Permalink /patch/216938/
State New
Headers show

Comments

Chris J Arges - Jan. 30, 2013, 3:22 p.m.
From: Takashi Iwai <tiwai@suse.de>

BugLink: http://bugs.launchpad.net/bugs/1097396

[NOTE: the regression below is found only in 3.2-3.4 stable trees, so
       there is no upstream commit corresponding to this patch]

The recent fix for the race at disconnection of usb-audio devices
(upstream commit 978520b7) triggers Oops when a device is unplugged
while playing on 3.2 and 3.4 kernels.  The culprit is that the
shutdown flag check was wrongly added around the urb deactivation code
snippet.  The urb deactivation code has to be performed even after the
device disconnected.  Otherwise it remains undead and pokes the wild
access in the end.

The regression fix is simply reverting the shutdown flag check in that
code.

Reported-and-tested-by: Chris J Arges <christopherarges@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

(cherry picked from commit 115b96e58609cff057d22d0e6118dae093763461)

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 sound/usb/endpoint.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
Chris J Arges - Jan. 30, 2013, 3:24 p.m.
The subject should have also contained [Precise SRU].
--chris

On 01/30/2013 09:22 AM, Chris J Arges wrote:
> From: Takashi Iwai <tiwai@suse.de>
> 
> BugLink: http://bugs.launchpad.net/bugs/1097396
> 
> [NOTE: the regression below is found only in 3.2-3.4 stable trees, so
>        there is no upstream commit corresponding to this patch]
> 
> The recent fix for the race at disconnection of usb-audio devices
> (upstream commit 978520b7) triggers Oops when a device is unplugged
> while playing on 3.2 and 3.4 kernels.  The culprit is that the
> shutdown flag check was wrongly added around the urb deactivation code
> snippet.  The urb deactivation code has to be performed even after the
> device disconnected.  Otherwise it remains undead and pokes the wild
> access in the end.
> 
> The regression fix is simply reverting the shutdown flag check in that
> code.
> 
> Reported-and-tested-by: Chris J Arges <christopherarges@gmail.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> (cherry picked from commit 115b96e58609cff057d22d0e6118dae093763461)
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> ---
>  sound/usb/endpoint.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 24c5114..9ab2b3e 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -148,10 +148,8 @@ void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
>  	int i;
>  
>  	/* stop urbs (to be sure) */
> -	if (!subs->stream->chip->shutdown) {
> -		deactivate_urbs(subs, force, 1);
> -		wait_clear_urbs(subs);
> -	}
> +	deactivate_urbs(subs, force, 1);
> +	wait_clear_urbs(subs);
>  
>  	for (i = 0; i < MAX_URBS; i++)
>  		release_urb_ctx(&subs->dataurb[i]);
>
Brad Figg - Jan. 30, 2013, 3:50 p.m.
On 01/30/2013 07:22 AM, Chris J Arges wrote:
> From: Takashi Iwai <tiwai@suse.de>
> 
> BugLink: http://bugs.launchpad.net/bugs/1097396
> 
> [NOTE: the regression below is found only in 3.2-3.4 stable trees, so
>        there is no upstream commit corresponding to this patch]
> 
> The recent fix for the race at disconnection of usb-audio devices
> (upstream commit 978520b7) triggers Oops when a device is unplugged
> while playing on 3.2 and 3.4 kernels.  The culprit is that the
> shutdown flag check was wrongly added around the urb deactivation code
> snippet.  The urb deactivation code has to be performed even after the
> device disconnected.  Otherwise it remains undead and pokes the wild
> access in the end.
> 
> The regression fix is simply reverting the shutdown flag check in that
> code.
> 
> Reported-and-tested-by: Chris J Arges <christopherarges@gmail.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> (cherry picked from commit 115b96e58609cff057d22d0e6118dae093763461)
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> ---
>  sound/usb/endpoint.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 24c5114..9ab2b3e 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -148,10 +148,8 @@ void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
>  	int i;
>  
>  	/* stop urbs (to be sure) */
> -	if (!subs->stream->chip->shutdown) {
> -		deactivate_urbs(subs, force, 1);
> -		wait_clear_urbs(subs);
> -	}
> +	deactivate_urbs(subs, force, 1);
> +	wait_clear_urbs(subs);
>  
>  	for (i = 0; i < MAX_URBS; i++)
>  		release_urb_ctx(&subs->dataurb[i]);
>
Herton Ronaldo Krzesinski - Jan. 30, 2013, 4:26 p.m.

Tim Gardner - Feb. 5, 2013, 4:21 p.m.

Patch

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 24c5114..9ab2b3e 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -148,10 +148,8 @@  void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
 	int i;
 
 	/* stop urbs (to be sure) */
-	if (!subs->stream->chip->shutdown) {
-		deactivate_urbs(subs, force, 1);
-		wait_clear_urbs(subs);
-	}
+	deactivate_urbs(subs, force, 1);
+	wait_clear_urbs(subs);
 
 	for (i = 0; i < MAX_URBS; i++)
 		release_urb_ctx(&subs->dataurb[i]);