[alsa-devel] [bug report] ALSA: pcm: Return -EBUSY for OSS ioctls changing busy streams

[Takashi Iwai]

On Tue, 27 Mar 2018 14:27:30 +0200,
Dan Carpenter wrote:
> 
> Hello Takashi Iwai,
> 
> The patch 40cab6e88cb0: "ALSA: pcm: Return -EBUSY for OSS ioctls
> changing busy streams" from Mar 23, 2018, leads to the following
> static checker warning:
> 
> 	sound/core/oss/pcm_oss.c:1741 snd_pcm_oss_set_rate()
> 	warn: inconsistent returns 'mutex:&runtime->oss.params_lock'.
> 	  Locked on:   line 1734
> 	  Unlocked on: line 1732
> 	               line 1741
> 
> sound/core/oss/pcm_oss.c
>   1717  static int snd_pcm_oss_set_rate(struct snd_pcm_oss_file *pcm_oss_file, int rate)
>   1718  {
>   1719          int idx;
>   1720  
>   1721          for (idx = 1; idx >= 0; --idx) {
>   1722                  struct snd_pcm_substream *substream = pcm_oss_file->streams[idx];
>   1723                  struct snd_pcm_runtime *runtime;
>   1724                  if (substream == NULL)
>   1725                          continue;
>   1726                  runtime = substream->runtime;
>   1727                  if (rate < 1000)
>   1728                          rate = 1000;
>   1729                  else if (rate > 192000)
>   1730                          rate = 192000;
>   1731                  if (mutex_lock_interruptible(&runtime->oss.params_lock))
>   1732                          return -ERESTARTSYS;
>   1733                  if (atomic_read(&runtime->oss.rw_ref))
>   1734                          return -EBUSY;
>                                 ^^^^^^^^^^^^^
> Unlock before returning?

My bad, it has to be checked before mutex_lock.
There is one another place doing the same mistake.

The fix patch is below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: pcm: Fix mutex unbalance in OSS emulation ioctls

The previous fix 40cab6e88cb0 ("ALSA: pcm: Return -EBUSY for OSS
ioctls changing busy streams") introduced some mutex unbalance, the
check of runtime->oss.rw_ref was inserted in a wrong place after the
mutex lock although it should have been done before the mutex lock.
Fix those spots.

Fixes: 40cab6e88cb0 ("ALSA: pcm: Return -EBUSY for OSS ioctls changing busy streams")
Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
Cc: <stable at vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/core/oss/pcm_oss.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index a9082f219561..b01611d1a5f0 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1728,10 +1728,10 @@ static int snd_pcm_oss_set_rate(struct snd_pcm_oss_file *pcm_oss_file, int rate)
 			rate = 1000;
 		else if (rate > 192000)
 			rate = 192000;
-		if (mutex_lock_interruptible(&runtime->oss.params_lock))
-			return -ERESTARTSYS;
 		if (atomic_read(&runtime->oss.rw_ref))
 			return -EBUSY;
+		if (mutex_lock_interruptible(&runtime->oss.params_lock))
+			return -ERESTARTSYS;
 		if (runtime->oss.rate != rate) {
 			runtime->oss.params = 1;
 			runtime->oss.rate = rate;
@@ -1764,10 +1764,10 @@ static int snd_pcm_oss_set_channels(struct snd_pcm_oss_file *pcm_oss_file, unsig
 		if (substream == NULL)
 			continue;
 		runtime = substream->runtime;
-		if (mutex_lock_interruptible(&runtime->oss.params_lock))
-			return -ERESTARTSYS;
 		if (atomic_read(&runtime->oss.rw_ref))
 			return -EBUSY;
+		if (mutex_lock_interruptible(&runtime->oss.params_lock))
+			return -ERESTARTSYS;
 		if (runtime->oss.channels != channels) {
 			runtime->oss.params = 1;
 			runtime->oss.channels = channels;
-- 
2.16.2