[PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

Srinivas Kandagatla posted 13 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens
Posted by Srinivas Kandagatla 1 week, 5 days ago
As prepare can be called mulitple times, this can result in multiple
graph opens for playback path.

This will result in a memory leaks, fix this by adding a check before
opening.

Fixes: be1fae62cf25 ("ASoC: q6apm-lpass-dai: close graph on prepare errors")
Cc: Stable@vger.kernel.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 5be37eeea329..ba64117b8cfe 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -181,7 +181,7 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
 	 * It is recommend to load DSP with source graph first and then sink
 	 * graph, so sequence for playback and capture will be different
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {
 		graph = q6apm_graph_open(dai->dev, NULL, dai->dev, graph_id);
 		if (IS_ERR(graph)) {
 			dev_err(dai->dev, "Failed to open graph (%d)\n", graph_id);
-- 
2.47.3
Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens
Posted by Mark Brown 1 week, 4 days ago
On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
> As prepare can be called mulitple times, this can result in multiple
> graph opens for playback path.

>  	 */
> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
bindings over that and now we're using the DAI ID to index into the
array (I didn't check for existing instances...).  This might be
impossible due to system design though.
Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens
Posted by Srinivas Kandagatla 1 week, 4 days ago

On 3/24/26 6:25 PM, Mark Brown wrote:
> On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
>> As prepare can be called mulitple times, this can result in multiple
>> graph opens for playback path.
> 
>>  	 */
>> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {
> 
> This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> bindings over that and now we're using the DAI ID to index into the

The driver has dai->id indexing the array in most places, and that is
how it has been for a while. This is one of the problem which last patch
is trying to address doing a check on the range. At somepoint we need to
move to dynamic allocation tbh.

--srini
> array (I didn't check for existing instances...).  This might be
> impossible due to system design though.
Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens
Posted by Mark Brown 1 week, 4 days ago
On Wed, Mar 25, 2026 at 11:33:45AM +0000, Srinivas Kandagatla wrote:
> On 3/24/26 6:25 PM, Mark Brown wrote:
> > On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:

> >> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

> > This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> > bindings over that and now we're using the DAI ID to index into the

> The driver has dai->id indexing the array in most places, and that is
> how it has been for a while. This is one of the problem which last patch
> is trying to address doing a check on the range. At somepoint we need to
> move to dynamic allocation tbh.

Yeah, I saw it was a bit shaky all over.  I think having the array size
bumps earlier might help at least make it clearer things are OK, but
dynamic structures of some kind would indeed be ideal.
Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens
Posted by Mark Brown 1 week, 4 days ago
On Tue, Mar 24, 2026 at 06:25:42PM +0000, Mark Brown wrote:
> On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
> > As prepare can be called mulitple times, this can result in multiple
> > graph opens for playback path.

> >  	 */
> > -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

> This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> bindings over that and now we're using the DAI ID to index into the
> array (I didn't check for existing instances...).  This might be
> impossible due to system design though.

Ah, found an update later in the series so it's OK in the end.  A
potential bisection issue but hopefully not the end of the world.