From dbe0a9a0f565e92cae153e6b40cafbff31cd829a Mon Sep 17 00:00:00 2001 From: Pratham Pratap Date: Sat, 9 May 2020 01:01:37 +0530 Subject: [PATCH] sound: usb: Ensure proper cleanup of uaudio_dev under all scenarios Consider a case where chip is freed before disabling the audio channel. This can happen when usb_audio_disconnect is called due to USB DevFS proc_disconnect_claim ioctl. usb_audio_disconnect will call uaudio_disconnect_cb which will wait for in_use to be false to cleanup the uaudio_dev. If in_use never becomes false and the wait_event is interrupted by some other signal then driver bails out esrly from here and doesn't cleanup the uaudio_dev. Since uaudio_dev_release is responsible for clearing the in_use based on stream disable call, fix the cyclic dependency here on uaudio_dev_release and uaudio_dev_cleanup by adding timeout in wait_event and allowing dev_cleanup to happen from uaudio_disconnect_cb. If disable stream request comes after this, handle_uaudio_stream_req will still go ahead and try to find substream of the card but will go to error path since card is already disconnected. This will set the return value to -ENODEV but in the error path driver is not checking for the correct return value and trying to access chip again. Fix this by adding one more check for -ENODEV in the error handling path. Change-Id: Ie11ad162f02c46878eb2663bf21cbafa54a62b0a Signed-off-by: Pratham Pratap --- sound/usb/usb_audio_qmi_svc.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/sound/usb/usb_audio_qmi_svc.c b/sound/usb/usb_audio_qmi_svc.c index ac05d6903df1..c3e5ea683081 100644 --- a/sound/usb/usb_audio_qmi_svc.c +++ b/sound/usb/usb_audio_qmi_svc.c @@ -33,6 +33,7 @@ #define BUS_INTERVAL_FULL_SPEED 1000 /* in us */ #define BUS_INTERVAL_HIGHSPEED_AND_ABOVE 125 /* in us */ #define MAX_BINTERVAL_ISOC_EP 16 +#define DEV_RELEASE_WAIT_TIMEOUT 10000 /* in ms */ #define SND_PCM_CARD_NUM_MASK 0xffff0000 #define SND_PCM_DEV_NUM_MASK 0xff00 @@ -929,12 +930,14 @@ static void uaudio_disconnect_cb(struct snd_usb_audio *chip) if (ret < 0) uaudio_err("qmi send failed with err: %d\n", ret); - ret = wait_event_interruptible(dev->disconnect_wq, - !atomic_read(&dev->in_use)); - if (ret < 0) { - uaudio_dbg("failed with ret %d\n", ret); - return; - } + ret = wait_event_interruptible_timeout(dev->disconnect_wq, + !atomic_read(&dev->in_use), + msecs_to_jiffies(DEV_RELEASE_WAIT_TIMEOUT)); + if (!ret) + uaudio_err("timeout while waiting for dev_release\n"); + else if (ret < 0) + uaudio_err("failed with ret %d\n", ret); + mutex_lock(&chip->dev_lock); } @@ -1199,13 +1202,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle, response: if (!req_msg->enable && ret != -EINVAL) { - if (info_idx >= 0) { - mutex_lock(&chip->dev_lock); - info = &uadev[pcm_card_num].info[info_idx]; - uaudio_dev_intf_cleanup(uadev[pcm_card_num].udev, info); - uaudio_dbg("release resources: intf# %d card# %d\n", - subs->interface, pcm_card_num); - mutex_unlock(&chip->dev_lock); + if (ret != -ENODEV) { + if (info_idx >= 0) { + mutex_lock(&chip->dev_lock); + info = &uadev[pcm_card_num].info[info_idx]; + uaudio_dev_intf_cleanup( + uadev[pcm_card_num].udev, + info); + uaudio_dbg("release resources: intf# %d card# %d\n", + subs->interface, pcm_card_num); + mutex_unlock(&chip->dev_lock); + } } if (atomic_read(&uadev[pcm_card_num].in_use)) kref_put(&uadev[pcm_card_num].kref,