From patchwork Fri Mar 23 17:09:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 10305153 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AD2B2600CC for ; Fri, 23 Mar 2018 17:09:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9AEB72903C for ; Fri, 23 Mar 2018 17:09:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8C7032903F; Fri, 23 Mar 2018 17:09:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 855D62903C for ; Fri, 23 Mar 2018 17:09:55 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 6CB00266D11; Fri, 23 Mar 2018 18:09:53 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id A3E3A266E6B; Fri, 23 Mar 2018 18:09:50 +0100 (CET) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 14F3D266D11 for ; Fri, 23 Mar 2018 18:09:47 +0100 (CET) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id BB63AAFCB for ; Fri, 23 Mar 2018 17:09:47 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Date: Fri, 23 Mar 2018 18:09:42 +0100 Message-Id: <20180323170942.18470-2-tiwai@suse.de> X-Mailer: git-send-email 2.16.2 In-Reply-To: <20180323170942.18470-1-tiwai@suse.de> References: <20180323170942.18470-1-tiwai@suse.de> Subject: [alsa-devel] [PATCH 2/2] ALSA: pcm: Return -EBUSY for OSS ioctls changing busy streams X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP OSS PCM stream management isn't modal but it allows ioctls issued at any time for changing the parameters. In the previous hardening patch ("ALSA: pcm: Avoid potential races between OSS ioctls and read/write"), we covered these races and prevent the corruption by protecting the concurrent accesses via params_lock mutex. However, this means that some ioctls that try to change the stream parameter (e.g. channels or format) would be blocked until the read/write finishes, and it may take really long. Basically changing the parameter while reading/writing is an invalid operation, hence it's even more user-friendly from the API POV if it returns -EBUSY in such a situation. This patch adds such checks in the relevant ioctls with the addition of read/write access refcount. Cc: Signed-off-by: Takashi Iwai --- include/sound/pcm_oss.h | 1 + sound/core/oss/pcm_oss.c | 36 +++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/include/sound/pcm_oss.h b/include/sound/pcm_oss.h index 760c969d885d..a5b5575c59dd 100644 --- a/include/sound/pcm_oss.h +++ b/include/sound/pcm_oss.h @@ -57,6 +57,7 @@ struct snd_pcm_oss_runtime { char *buffer; /* vmallocated period */ size_t buffer_used; /* used length from period buffer */ struct mutex params_lock; + atomic_t rw_ref; /* concurrent read/write accesses */ #ifdef CONFIG_SND_PCM_OSS_PLUGINS struct snd_pcm_plugin *plugin_first; struct snd_pcm_plugin *plugin_last; diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index f8bfee8022e0..a9082f219561 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -1370,6 +1370,7 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha if (atomic_read(&substream->mmap_count)) return -ENXIO; + atomic_inc(&runtime->oss.rw_ref); while (bytes > 0) { if (mutex_lock_interruptible(&runtime->oss.params_lock)) { tmp = -ERESTARTSYS; @@ -1433,6 +1434,7 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha } tmp = 0; } + atomic_dec(&runtime->oss.rw_ref); return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp; } @@ -1478,6 +1480,7 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use if (atomic_read(&substream->mmap_count)) return -ENXIO; + atomic_inc(&runtime->oss.rw_ref); while (bytes > 0) { if (mutex_lock_interruptible(&runtime->oss.params_lock)) { tmp = -ERESTARTSYS; @@ -1526,6 +1529,7 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use } tmp = 0; } + atomic_dec(&runtime->oss.rw_ref); return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp; } @@ -1632,8 +1636,11 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) goto __direct; if ((err = snd_pcm_oss_make_ready(substream)) < 0) return err; - if (mutex_lock_interruptible(&runtime->oss.params_lock)) + atomic_inc(&runtime->oss.rw_ref); + if (mutex_lock_interruptible(&runtime->oss.params_lock)) { + atomic_dec(&runtime->oss.rw_ref); return -ERESTARTSYS; + } format = snd_pcm_oss_format_from(runtime->oss.format); width = snd_pcm_format_physical_width(format); if (runtime->oss.buffer_used > 0) { @@ -1645,10 +1652,8 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) runtime->oss.buffer + runtime->oss.buffer_used, size); err = snd_pcm_oss_sync1(substream, runtime->oss.period_bytes); - if (err < 0) { - mutex_unlock(&runtime->oss.params_lock); - return err; - } + if (err < 0) + goto unlock; } else if (runtime->oss.period_ptr > 0) { #ifdef OSS_DEBUG pcm_dbg(substream->pcm, "sync: period_ptr\n"); @@ -1658,10 +1663,8 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) runtime->oss.buffer, size * 8 / width); err = snd_pcm_oss_sync1(substream, size); - if (err < 0) { - mutex_unlock(&runtime->oss.params_lock); - return err; - } + if (err < 0) + goto unlock; } /* * The ALSA's period might be a bit large than OSS one. @@ -1675,7 +1678,11 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) else if (runtime->access == SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) snd_pcm_lib_writev(substream, NULL, size); } +unlock: mutex_unlock(&runtime->oss.params_lock); + atomic_dec(&runtime->oss.rw_ref); + if (err < 0) + return err; /* * finish sync: drain the buffer */ @@ -1723,6 +1730,8 @@ static int snd_pcm_oss_set_rate(struct snd_pcm_oss_file *pcm_oss_file, int rate) rate = 192000; if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -ERESTARTSYS; + if (atomic_read(&runtime->oss.rw_ref)) + return -EBUSY; if (runtime->oss.rate != rate) { runtime->oss.params = 1; runtime->oss.rate = rate; @@ -1757,6 +1766,8 @@ static int snd_pcm_oss_set_channels(struct snd_pcm_oss_file *pcm_oss_file, unsig runtime = substream->runtime; if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -ERESTARTSYS; + if (atomic_read(&runtime->oss.rw_ref)) + return -EBUSY; if (runtime->oss.channels != channels) { runtime->oss.params = 1; runtime->oss.channels = channels; @@ -1847,6 +1858,8 @@ static int snd_pcm_oss_set_format(struct snd_pcm_oss_file *pcm_oss_file, int for if (substream == NULL) continue; runtime = substream->runtime; + if (atomic_read(&runtime->oss.rw_ref)) + return -EBUSY; if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -ERESTARTSYS; if (runtime->oss.format != format) { @@ -1901,6 +1914,8 @@ static int snd_pcm_oss_set_subdivide(struct snd_pcm_oss_file *pcm_oss_file, int if (substream == NULL) continue; runtime = substream->runtime; + if (atomic_read(&runtime->oss.rw_ref)) + return -EBUSY; if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -ERESTARTSYS; err = snd_pcm_oss_set_subdivide1(substream, subdivide); @@ -1939,6 +1954,8 @@ static int snd_pcm_oss_set_fragment(struct snd_pcm_oss_file *pcm_oss_file, unsig if (substream == NULL) continue; runtime = substream->runtime; + if (atomic_read(&runtime->oss.rw_ref)) + return -EBUSY; if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -ERESTARTSYS; err = snd_pcm_oss_set_fragment1(substream, val); @@ -2333,6 +2350,7 @@ static void snd_pcm_oss_init_substream(struct snd_pcm_substream *substream, runtime->oss.maxfrags = 0; runtime->oss.subdivision = 0; substream->pcm_release = snd_pcm_oss_release_substream; + atomic_set(&runtime->oss.rw_ref, 0); } static int snd_pcm_oss_release_file(struct snd_pcm_oss_file *pcm_oss_file)