Re: [PATCH v2 19/25] ASoC: intel: atom: Fix .shutdown() being skipped on rollback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ALSA: intel: atom: resource leak on sst_media_open() error path
--

commit fac36234f535190efd5d6e1b1740986dc014a4d6
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: intel: atom: Fix .shutdown() being skipped on rollback

snd_pcm_hw_constraint_integer() may return 1, which is then returned
from sst_media_open(). ASoC only marks the DAI as started when
.startup() returns 0, so .shutdown() can be skipped on rollback.
This commit returns 0 explicitly on success.

> diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> index a4c8cbfba0963..58681e007abf9 100644
> --- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> +++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> @@ -341,7 +341,12 @@ static int sst_media_open(struct snd_pcm_substream *substream,
>  
>  	stream = NULL;
>  
> -	return ret_val;
> +	/*
> +	 * ASoC only marks the DAI as started when .startup() returns 0.
> +	 * Since snd_pcm_hw_constraint_integer() may return 1, explicitly
> +	 * return 0 to ensure .shutdown() is not skipped on rollback.
> +	 */
> +	return 0;
>  }

[Severity: High]
Since ASoC only marks the DAI as started when .startup() returns 0, do the
error paths earlier in sst_media_open() leak resources?

For example, if power_up_sst() fails:

	ret_val = power_up_sst(stream);
	if (ret_val < 0)
		return ret_val;

The function returns the error directly. Since .startup() returns an error,
.shutdown() won't be called. Does this leak the module reference acquired
earlier via try_module_get(sst->dev->driver->owner)?

Similarly, if snd_pcm_hw_constraint_integer() fails:

	ret_val = snd_pcm_hw_constraint_integer(runtime,
						SNDRV_PCM_HW_PARAM_PERIODS);

	if (ret_val < 0)
		return ret_val;

The function returns the error, skipping .shutdown(). Does this leak both
the module reference and the power state from power_up_sst(stream)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015014324.5569-1-phucduc.bui@gmail.com?part=19