[PATCH] ALSA: seq: Correct unlock sequence at snd_seq_client_ioctl_unlock()

[PATCH] ALSA: seq: Correct unlock sequence at snd_seq_client_ioctl_unlock()

[Takashi Iwai]

The doubly unlock sequence at snd_seq_client_ioctl_unlock() is tricky.
I took a direct unref call since I thought it would avoid
misunderstanding, but rather this seems more confusing.  Let's use
snd_seq_client_unlock() consistently even if they look strange to be
called twice, and add more comments for avoiding reader's confusion.

Fixes: 6b580f523172 ("ALSA: seq: Protect racy pool manipulation from OSS sequencer")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 3acd80e718f2..c0227a672442 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -191,6 +191,7 @@ bool snd_seq_client_ioctl_lock(int clientid)
 	if (!client)
 		return false;
 	mutex_lock(&client->ioctl_mutex);
+	/* The client isn't unrefed here; see snd_seq_client_ioctl_unlock() */
 	return true;
 }
 EXPORT_SYMBOL_GPL(snd_seq_client_ioctl_lock);
@@ -204,7 +205,11 @@ void snd_seq_client_ioctl_unlock(int clientid)
 	if (WARN_ON(!client))
 		return;
 	mutex_unlock(&client->ioctl_mutex);
-	snd_use_lock_free(&client->use_lock);
+	/* The doubly unrefs below are intentional; the first one releases the
+	 * leftover from snd_seq_client_ioctl_lock() above, and the second one
+	 * is for releasing snd_seq_client_use_ptr() in this function
+	 */
+	snd_seq_client_unlock(client);
 	snd_seq_client_unlock(client);
 }
 EXPORT_SYMBOL_GPL(snd_seq_client_ioctl_unlock);
-- 
2.16.4

Re: [PATCH] ALSA: seq: Correct unlock sequence at snd_seq_client_ioctl_unlock()

[Kai Vehmanen]

Hi,

On Mon, 15 Apr 2019, Takashi Iwai wrote:

> The doubly unlock sequence at snd_seq_client_ioctl_unlock() is tricky.
> I took a direct unref call since I thought it would avoid
> misunderstanding, but rather this seems more confusing.  Let's use
> snd_seq_client_unlock() consistently even if they look strange to be
> called twice, and add more comments for avoiding reader's confusion.
> 
> Fixes: 6b580f523172 ("ALSA: seq: Protect racy pool manipulation from OSS sequencer")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/seq/seq_clientmgr.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

thanks, looks good now (this and rest of the clean-up series):
  Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Br, Kai

Re: [PATCH] ALSA: seq: Correct unlock sequence at snd_seq_client_ioctl_unlock()

[Takashi Iwai]

On Mon, 15 Apr 2019 12:05:07 +0200,
Kai Vehmanen wrote:
> 
> Hi,
> 
> On Mon, 15 Apr 2019, Takashi Iwai wrote:
> 
> > The doubly unlock sequence at snd_seq_client_ioctl_unlock() is tricky.
> > I took a direct unref call since I thought it would avoid
> > misunderstanding, but rather this seems more confusing.  Let's use
> > snd_seq_client_unlock() consistently even if they look strange to be
> > called twice, and add more comments for avoiding reader's confusion.
> > 
> > Fixes: 6b580f523172 ("ALSA: seq: Protect racy pool manipulation from OSS sequencer")
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/core/seq/seq_clientmgr.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> thanks, looks good now (this and rest of the clean-up series):
>   Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Thanks!


Takashi