From patchwork Fri May 13 14:23:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 572936 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 527C5C433FE for ; Fri, 13 May 2022 14:27:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1380701AbiEMO1d (ORCPT ); Fri, 13 May 2022 10:27:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39848 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1381081AbiEMO0a (ORCPT ); Fri, 13 May 2022 10:26:30 -0400 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7A06D5F8F6; Fri, 13 May 2022 07:26:16 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id EA792CE3233; Fri, 13 May 2022 14:26:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 08279C34115; Fri, 13 May 2022 14:26:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1652451973; bh=oBP614t9fTptQhQswUAjX6Y5Y8f75tmsz/oYWzjS59U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LE6nH1veG0f4SNQ9252awCRD6qxWbCnV2ZkvS3nFG9PZdJeEUtVaX1wAFy+fVuo9G yIcipi/CvI1LSkrfu+oFSFc3yqcOe45eTX9eJlJPVTlFyatHJScbg1SLvJlJKogsM1 PFsLSALFa0HnCo5Da+Kdb/LwLplsTG9NWyToBeJ8= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , Hu Jiahui , Jaroslav Kysela , Takashi Iwai , Ovidiu Panait Subject: [PATCH 4.19 08/15] ALSA: pcm: Fix races among concurrent hw_params and hw_free calls Date: Fri, 13 May 2022 16:23:30 +0200 Message-Id: <20220513142228.140882635@linuxfoundation.org> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220513142227.897535454@linuxfoundation.org> References: <20220513142227.897535454@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Takashi Iwai commit 92ee3c60ec9fe64404dc035e7c41277d74aa26cb upstream. Currently we have neither proper check nor protection against the concurrent calls of PCM hw_params and hw_free ioctls, which may result in a UAF. Since the existing PCM stream lock can't be used for protecting the whole ioctl operations, we need a new mutex to protect those racy calls. This patch introduced a new mutex, runtime->buffer_mutex, and applies it to both hw_params and hw_free ioctl code paths. Along with it, the both functions are slightly modified (the mmap_count check is moved into the state-check block) for code simplicity. Reported-by: Hu Jiahui Cc: Reviewed-by: Jaroslav Kysela Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de Signed-off-by: Takashi Iwai [OP: backport to 4.19: adjusted context] Signed-off-by: Ovidiu Panait Signed-off-by: Greg Kroah-Hartman --- include/sound/pcm.h | 1 sound/core/pcm.c | 2 + sound/core/pcm_native.c | 55 +++++++++++++++++++++++++++++++----------------- 3 files changed, 39 insertions(+), 19 deletions(-) --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -404,6 +404,7 @@ struct snd_pcm_runtime { wait_queue_head_t sleep; /* poll sleep */ wait_queue_head_t tsleep; /* transfer sleep */ struct fasync_struct *fasync; + struct mutex buffer_mutex; /* protect for buffer changes */ /* -- private section -- */ void *private_data; --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1031,6 +1031,7 @@ int snd_pcm_attach_substream(struct snd_ init_waitqueue_head(&runtime->tsleep); runtime->status->state = SNDRV_PCM_STATE_OPEN; + mutex_init(&runtime->buffer_mutex); substream->runtime = runtime; substream->private_data = pcm->private_data; @@ -1062,6 +1063,7 @@ void snd_pcm_detach_substream(struct snd substream->runtime = NULL; if (substream->timer) spin_unlock_irq(&substream->timer->lock); + mutex_destroy(&runtime->buffer_mutex); kfree(runtime); put_pid(substream->pid); substream->pid = NULL; --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -666,33 +666,40 @@ static int snd_pcm_hw_params_choose(stru return 0; } +#if IS_ENABLED(CONFIG_SND_PCM_OSS) +#define is_oss_stream(substream) ((substream)->oss.oss) +#else +#define is_oss_stream(substream) false +#endif + static int snd_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_pcm_runtime *runtime; - int err, usecs; + int err = 0, usecs; unsigned int bits; snd_pcm_uframes_t frames; if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; + mutex_lock(&runtime->buffer_mutex); snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_OPEN: case SNDRV_PCM_STATE_SETUP: case SNDRV_PCM_STATE_PREPARED: + if (!is_oss_stream(substream) && + atomic_read(&substream->mmap_count)) + err = -EBADFD; break; default: - snd_pcm_stream_unlock_irq(substream); - return -EBADFD; + err = -EBADFD; + break; } snd_pcm_stream_unlock_irq(substream); -#if IS_ENABLED(CONFIG_SND_PCM_OSS) - if (!substream->oss.oss) -#endif - if (atomic_read(&substream->mmap_count)) - return -EBADFD; + if (err) + goto unlock; params->rmask = ~0U; err = snd_pcm_hw_refine(substream, params); @@ -769,14 +776,19 @@ static int snd_pcm_hw_params(struct snd_ if ((usecs = period_to_usecs(runtime)) >= 0) pm_qos_add_request(&substream->latency_pm_qos_req, PM_QOS_CPU_DMA_LATENCY, usecs); - return 0; + err = 0; _error: - /* hardware might be unusable from this time, - so we force application to retry to set - the correct hardware parameter settings */ - snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); - if (substream->ops->hw_free != NULL) - substream->ops->hw_free(substream); + if (err) { + /* hardware might be unusable from this time, + * so we force application to retry to set + * the correct hardware parameter settings + */ + snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); + if (substream->ops->hw_free != NULL) + substream->ops->hw_free(substream); + } + unlock: + mutex_unlock(&runtime->buffer_mutex); return err; } @@ -809,22 +821,27 @@ static int snd_pcm_hw_free(struct snd_pc if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; + mutex_lock(&runtime->buffer_mutex); snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_SETUP: case SNDRV_PCM_STATE_PREPARED: + if (atomic_read(&substream->mmap_count)) + result = -EBADFD; break; default: - snd_pcm_stream_unlock_irq(substream); - return -EBADFD; + result = -EBADFD; + break; } snd_pcm_stream_unlock_irq(substream); - if (atomic_read(&substream->mmap_count)) - return -EBADFD; + if (result) + goto unlock; if (substream->ops->hw_free) result = substream->ops->hw_free(substream); snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); pm_qos_remove_request(&substream->latency_pm_qos_req); + unlock: + mutex_unlock(&runtime->buffer_mutex); return result; }