Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760707AbaGDWWd (ORCPT ); Fri, 4 Jul 2014 18:22:33 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:57659 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760671AbaGDWW0 (ORCPT ); Fri, 4 Jul 2014 18:22:26 -0400 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Alan Stern , Takashi Iwai Subject: [PATCH 3.14 57/59] ALSA: usb-audio: Fix races at disconnection and PCM closing Date: Fri, 4 Jul 2014 15:19:46 -0700 Message-Id: <20140704221542.239710678@linuxfoundation.org> X-Mailer: git-send-email 2.0.1 In-Reply-To: <20140704221539.544876024@linuxfoundation.org> References: <20140704221539.544876024@linuxfoundation.org> User-Agent: quilt/0.63-1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 3.14-stable review patch. If anyone has any objections, please let me know. ------------------ From: Takashi Iwai commit 92a586bdc06de6629dae1b357dac221253f55ff8 upstream. When a USB-audio device is disconnected while PCM is still running, we still see some race: the disconnect callback calls snd_usb_endpoint_free() that calls release_urbs() and then kfree() while a PCM stream would be closed at the same time and calls stop_endpoints() that leads to wait_clear_urbs(). That is, the EP object might be deallocated while a PCM stream is syncing with wait_clear_urbs() with the same EP. Basically calling multiple wait_clear_urbs() would work fine, also calling wait_clear_urbs() and release_urbs() would work, too, as wait_clear_urbs() just reads some fields in ep. The problem is the succeeding kfree() in snd_pcm_endpoint_free(). This patch moves out the EP deallocation into the later point, the destructor callback. At this stage, all PCMs must have been already closed, so it's safe to free the objects. Reported-by: Alan Stern Signed-off-by: Takashi Iwai Signed-off-by: Greg Kroah-Hartman --- sound/usb/card.c | 13 ++++++++++--- sound/usb/endpoint.c | 17 ++++++++++++++--- sound/usb/endpoint.h | 1 + 3 files changed, 25 insertions(+), 6 deletions(-) --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -304,6 +304,11 @@ static int snd_usb_create_streams(struct static int snd_usb_audio_free(struct snd_usb_audio *chip) { + struct list_head *p, *n; + + list_for_each_safe(p, n, &chip->ep_list) + snd_usb_endpoint_free(p); + mutex_destroy(&chip->mutex); kfree(chip); return 0; @@ -580,7 +585,7 @@ static void snd_usb_audio_disconnect(str struct snd_usb_audio *chip) { struct snd_card *card; - struct list_head *p, *n; + struct list_head *p; if (chip == (void *)-1L) return; @@ -593,14 +598,16 @@ static void snd_usb_audio_disconnect(str mutex_lock(®ister_mutex); chip->num_interfaces--; if (chip->num_interfaces <= 0) { + struct snd_usb_endpoint *ep; + snd_card_disconnect(card); /* release the pcm resources */ list_for_each(p, &chip->pcm_list) { snd_usb_stream_disconnect(p); } /* release the endpoint resources */ - list_for_each_safe(p, n, &chip->ep_list) { - snd_usb_endpoint_free(p); + list_for_each_entry(ep, &chip->ep_list, list) { + snd_usb_endpoint_release(ep); } /* release the midi resources */ list_for_each(p, &chip->midi_list) { --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -981,19 +981,30 @@ void snd_usb_endpoint_deactivate(struct } /** + * snd_usb_endpoint_release: Tear down an snd_usb_endpoint + * + * @ep: the endpoint to release + * + * This function does not care for the endpoint's use count but will tear + * down all the streaming URBs immediately. + */ +void snd_usb_endpoint_release(struct snd_usb_endpoint *ep) +{ + release_urbs(ep, 1); +} + +/** * snd_usb_endpoint_free: Free the resources of an snd_usb_endpoint * * @ep: the list header of the endpoint to free * - * This function does not care for the endpoint's use count but will tear - * down all the streaming URBs immediately and free all resources. + * This free all resources of the given ep. */ void snd_usb_endpoint_free(struct list_head *head) { struct snd_usb_endpoint *ep; ep = list_entry(head, struct snd_usb_endpoint, list); - release_urbs(ep, 1); kfree(ep); } --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -23,6 +23,7 @@ void snd_usb_endpoint_stop(struct snd_us void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep); +void snd_usb_endpoint_release(struct snd_usb_endpoint *ep); void snd_usb_endpoint_free(struct list_head *head); int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/