Re: [PATCH v2 18/25] ASoC: imx-pcm-rpmsg: Fix imx_rpmsg_pcm_close() being skipped on rollback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: fsl: rpmsg: remote state leak on hw_constraint error
--

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

ASoC: imx-pcm-rpmsg: Fix imx_rpmsg_pcm_close() being skipped on rollback

This commit fixes an issue where the close callback was being skipped
when snd_pcm_hw_constraint_integer() returned a positive value. It ensures
the component is properly marked as opened by explicitly returning 0 on success.

> diff --git a/sound/soc/fsl/imx-pcm-rpmsg.c b/sound/soc/fsl/imx-pcm-rpmsg.c
> index 7086fb7fc1eba..85b7c929a28a4 100644
> --- a/sound/soc/fsl/imx-pcm-rpmsg.c
> +++ b/sound/soc/fsl/imx-pcm-rpmsg.c
> @@ -269,7 +269,13 @@ static int imx_rpmsg_pcm_open(struct snd_soc_component *component,
>  	info->stream_timer[substream->stream].substream = substream;
>  	timer_setup(&info->stream_timer[substream->stream].timer,
>  		    imx_rpmsg_timer_callback, 0);
> -	return ret;
> +
> +	/*
> +	 * ASoC only marks the component as opened when .open() returns 0.
> +	 * Since snd_pcm_hw_constraint_integer() may return 1, explicitly
> +	 * return 0 to ensure .close() is not skipped on rollback.
> +	 */
> +	return 0;
>  }

[Severity: Medium]
If snd_pcm_hw_constraint_integer() returns an error, can this still leave the
remote DSP in a leaked state?

Earlier in imx_rpmsg_pcm_open(), an open message is successfully sent to the
remote DSP:

sound/soc/fsl/imx-pcm-rpmsg.c:imx_rpmsg_pcm_open() {
	...
	info->send_message(msg, info);
	...
	ret = snd_pcm_hw_constraint_integer(substream->runtime,
					    SNDRV_PCM_HW_PARAM_PERIODS);
	if (ret < 0)
		return ret;
	...
}

Because returning a negative error code prevents ASoC from marking the
component as opened, snd_soc_component_close() will skip calling the close
callback on error rollback. 

Does the driver need to manually send a close message before returning the
error, or is there another way to ensure the remote DSP state is cleaned up
on this failure path?

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