Re: sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179)

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: kbuild-all@lists.01.org
Subject: Re: sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179)
Date: Mon, 09 Dec 2019 08:24:38 +0900	[thread overview]
Message-ID: <20191208232437.GA6741@workstation> (raw)
In-Reply-To: <20191208084034.GU1787@kadam>

[-- Attachment #1: Type: text/plain, Size: 6308 bytes --]

Hi Dan,

On Sun, Dec 08, 2019 at 11:40:34AM +0300, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   94e89b40235476a83a53a47b9ffb0cb91a4c335e
> commit: 3fd80b2003882b6a328caff9e6b3a14bed61f27c ALSA: firewire-motu: use the same size of period for PCM substream in AMDTP streams
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179)

Indeed, it's a bug introduced in v5.5 kernel. I posted a patch to fix
this just now:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-December/159552.html

> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3fd80b2003882b6a328caff9e6b3a14bed61f27c
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git remote update linus
> git checkout 3fd80b2003882b6a328caff9e6b3a14bed61f27c
> vim +191 sound/firewire/motu/motu-pcm.c
> 
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  133  static int pcm_open(struct snd_pcm_substream *substream)
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  134  {
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  135  	struct snd_motu *motu = substream->private_data;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  136  	const struct snd_motu_protocol *const protocol = motu->spec->protocol;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  137  	struct amdtp_domain *d = &motu->domain;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  138  	enum snd_motu_clock_source src;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  139  	int err;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  140  
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  141  	err = snd_motu_stream_lock_try(motu);
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  142  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  143  		return err;
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  144  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  145  	mutex_lock(&motu->mutex);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  146  
> 8b460c76bd1712 Takashi Sakamoto 2017-08-20  147  	err = snd_motu_stream_cache_packet_formats(motu);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  148  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  149  		goto err_locked;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  150  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  151  	err = init_hw_info(motu, substream);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  152  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  153  		goto err_locked;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  154  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  155  	err = protocol->get_clock_source(motu, &src);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  156  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  157  		goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  158  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  159  	// When source of clock is not internal or any stream is reserved for
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  160  	// transmission of PCM frames, the available sampling rate is limited
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  161  	// at current one.
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  162  	if (src != SND_MOTU_CLOCK_SOURCE_INTERNAL ||
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  163  	    (motu->substreams_counter > 0 && d->events_per_period > 0)) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  164  		unsigned int frames_per_period = d->events_per_period;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  165  		unsigned int rate;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  166  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  167  		err = protocol->get_clock_rate(motu, &rate);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  168  		if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  169  			goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  170  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  171  		substream->runtime->hw.rate_min = rate;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  172  		substream->runtime->hw.rate_max = rate;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  173  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  174  		if (frames_per_period > 0) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  175  			err = snd_pcm_hw_constraint_minmax(substream->runtime,
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  176  					SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  177  					frames_per_period, frames_per_period);
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  178  			if (err < 0) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07 @179  				mutex_unlock(&motu->mutex);
>                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Delete this.
> 
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  180  				goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  181  			}
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  182  		}
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  183  	}
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  184  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  185  	snd_pcm_set_sync(substream);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  186  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  187  	mutex_unlock(&motu->mutex);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  188  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  189  	return 0;
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  190  err_locked:
> 71c3797779d3cd Takashi Sakamoto 2017-03-22 @191  	mutex_unlock(&motu->mutex);
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  192  	snd_motu_stream_lock_release(motu);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  193  	return err;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  194  }
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  195  
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation


Thanks

Takashi Sakamoto

WARNING: multiple messages have this Message-ID (diff)

From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: kbuild@lists.01.org, kbuild-all@lists.01.org,
	linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.de>
Subject: Re: sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179)
Date: Mon, 9 Dec 2019 08:24:38 +0900	[thread overview]
Message-ID: <20191208232437.GA6741@workstation> (raw)
In-Reply-To: <20191208084034.GU1787@kadam>

Hi Dan,

On Sun, Dec 08, 2019 at 11:40:34AM +0300, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   94e89b40235476a83a53a47b9ffb0cb91a4c335e
> commit: 3fd80b2003882b6a328caff9e6b3a14bed61f27c ALSA: firewire-motu: use the same size of period for PCM substream in AMDTP streams
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179)

Indeed, it's a bug introduced in v5.5 kernel. I posted a patch to fix
this just now:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-December/159552.html

> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3fd80b2003882b6a328caff9e6b3a14bed61f27c
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git remote update linus
> git checkout 3fd80b2003882b6a328caff9e6b3a14bed61f27c
> vim +191 sound/firewire/motu/motu-pcm.c
> 
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  133  static int pcm_open(struct snd_pcm_substream *substream)
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  134  {
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  135  	struct snd_motu *motu = substream->private_data;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  136  	const struct snd_motu_protocol *const protocol = motu->spec->protocol;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  137  	struct amdtp_domain *d = &motu->domain;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  138  	enum snd_motu_clock_source src;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  139  	int err;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  140  
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  141  	err = snd_motu_stream_lock_try(motu);
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  142  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  143  		return err;
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  144  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  145  	mutex_lock(&motu->mutex);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  146  
> 8b460c76bd1712 Takashi Sakamoto 2017-08-20  147  	err = snd_motu_stream_cache_packet_formats(motu);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  148  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  149  		goto err_locked;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  150  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  151  	err = init_hw_info(motu, substream);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  152  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  153  		goto err_locked;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  154  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  155  	err = protocol->get_clock_source(motu, &src);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  156  	if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  157  		goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  158  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  159  	// When source of clock is not internal or any stream is reserved for
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  160  	// transmission of PCM frames, the available sampling rate is limited
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  161  	// at current one.
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  162  	if (src != SND_MOTU_CLOCK_SOURCE_INTERNAL ||
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  163  	    (motu->substreams_counter > 0 && d->events_per_period > 0)) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  164  		unsigned int frames_per_period = d->events_per_period;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  165  		unsigned int rate;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  166  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  167  		err = protocol->get_clock_rate(motu, &rate);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  168  		if (err < 0)
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  169  			goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  170  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  171  		substream->runtime->hw.rate_min = rate;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  172  		substream->runtime->hw.rate_max = rate;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  173  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  174  		if (frames_per_period > 0) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  175  			err = snd_pcm_hw_constraint_minmax(substream->runtime,
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  176  					SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  177  					frames_per_period, frames_per_period);
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  178  			if (err < 0) {
> 3fd80b2003882b Takashi Sakamoto 2019-10-07 @179  				mutex_unlock(&motu->mutex);
>                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Delete this.
> 
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  180  				goto err_locked;
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  181  			}
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  182  		}
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  183  	}
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  184  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  185  	snd_pcm_set_sync(substream);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  186  
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  187  	mutex_unlock(&motu->mutex);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  188  
> 3fd80b2003882b Takashi Sakamoto 2019-10-07  189  	return 0;
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  190  err_locked:
> 71c3797779d3cd Takashi Sakamoto 2017-03-22 @191  	mutex_unlock(&motu->mutex);
> 71c3797779d3cd Takashi Sakamoto 2017-03-22  192  	snd_motu_stream_lock_release(motu);
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  193  	return err;
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  194  }
> dd49b2d1f04af9 Takashi Sakamoto 2017-03-22  195  
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation


Thanks

Takashi Sakamoto

next prev parent reply	other threads:[~2019-12-08 23:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-08  8:40 sound/firewire/motu/motu-pcm.c:191 pcm_open() error: double unlocked 'motu->mutex' (orig line 179) Dan Carpenter
2019-12-08  8:40 ` Dan Carpenter
2019-12-08  8:40 ` Dan Carpenter
2019-12-08 23:24 ` Takashi Sakamoto [this message]
2019-12-08 23:24   ` Takashi Sakamoto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191208232437.GA6741@workstation \
    --to=o-takashi@sakamocchi.jp \
    --cc=kbuild-all@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.