linux-sound.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed
@ 2025-08-15 11:39 Krzysztof Kozlowski
  2025-08-15 15:56 ` Srinivas Kandagatla
  2025-09-04 16:51 ` Mark Brown
  0 siblings, 2 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-15 11:39 UTC (permalink / raw)
  To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Pierre-Louis Bossart, linux-sound, linux-arm-msm,
	linux-kernel
  Cc: Krzysztof Kozlowski, stable

If earlier opening of source graph fails (e.g. ADSP rejects due to
incorrect audioreach topology), the graph is closed and
"dai_data->graph[dai->id]" is assigned NULL.  Preparing the DAI for sink
graph continues though and next call to q6apm_lpass_dai_prepare()
receives dai_data->graph[dai->id]=NULL leading to NULL pointer
exception:

  qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
  qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
  q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: fail to start APM port 78
  q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC: error at snd_soc_pcm_dai_prepare on TX_CODEC_DMA_TX_3: -22
  Unable to handle kernel NULL pointer dereference at virtual address 00000000000000a8
  ...
  Call trace:
   q6apm_graph_media_format_pcm+0x48/0x120 (P)
   q6apm_lpass_dai_prepare+0x110/0x1b4
   snd_soc_pcm_dai_prepare+0x74/0x108
   __soc_pcm_prepare+0x44/0x160
   dpcm_be_dai_prepare+0x124/0x1c0

Fixes: 30ad723b93ad ("ASoC: qdsp6: audioreach: add q6apm lpass dai support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index f90628d9b90e..7520e6f024c3 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -191,6 +191,12 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
 			return rc;
 		}
 		dai_data->graph[graph_id] = graph;
+	} else if (!dai_data->graph[dai->id]) {
+		/*
+		 * Loading source graph failed before, so abort loading the sink
+		 * as well.
+		 */
+		return -EINVAL;
 	}
 
 	cfg->direction = substream->stream;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed
  2025-08-15 11:39 [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed Krzysztof Kozlowski
@ 2025-08-15 15:56 ` Srinivas Kandagatla
  2025-08-18 11:47   ` Krzysztof Kozlowski
  2025-09-04 16:51 ` Mark Brown
  1 sibling, 1 reply; 4+ messages in thread
From: Srinivas Kandagatla @ 2025-08-15 15:56 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Srinivas Kandagatla, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart,
	linux-sound, linux-arm-msm, linux-kernel
  Cc: stable

Thanks Krzysztof,
On 8/15/25 12:39 PM, Krzysztof Kozlowski wrote:
> If earlier opening of source graph fails (e.g. ADSP rejects due to

I think you are referring to the err patch in prepare.

> incorrect audioreach topology), the graph is closed and
> "dai_data->graph[dai->id]" is assigned NULL.  Preparing the DAI for sink
> graph continues though and next call to q6apm_lpass_dai_prepare()
> receives dai_data->graph[dai->id]=NULL leading to NULL pointer
> exception:
> 
>   qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>   qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>   q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: fail to start APM port 78
>   q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC: error at snd_soc_pcm_dai_prepare on TX_CODEC_DMA_TX_3: -22
>   Unable to handle kernel NULL pointer dereference at virtual address 00000000000000a8
>   ...
>   Call trace:
>    q6apm_graph_media_format_pcm+0x48/0x120 (P)
>    q6apm_lpass_dai_prepare+0x110/0x1b4
>    snd_soc_pcm_dai_prepare+0x74/0x108
>    __soc_pcm_prepare+0x44/0x160
>    dpcm_be_dai_prepare+0x124/0x1c0
> 
> Fixes: 30ad723b93ad ("ASoC: qdsp6: audioreach: add q6apm lpass dai support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> index f90628d9b90e..7520e6f024c3 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> @@ -191,6 +191,12 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
>  			return rc;
>  		}
>  		dai_data->graph[graph_id] = graph;
> +	} else if (!dai_data->graph[dai->id]) {
> +		/*
> +		 * Loading source graph failed before, so abort loading the sink
> +		 * as well.
> +		 */
> +		return -EINVAL;
>  	}
I guess this is the capture graph that is triggering the error, normally
we do not open/close the capture graph in prepare, we do
stop/prepare/start for capture graphs and handle open close in
startup/shutdown.

Can you try this change and see if it fixes the issue, as prepare could
be called multiple times and your patch will not give chance for trying
new parameters incase the failure was due to unsupported params.


--------------------->cut<------------------------------
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index a0d90462fd6a..20974f10406b 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -213,8 +213,10 @@ static int q6apm_lpass_dai_prepare(struct
snd_pcm_substream *substream, struct s

        return 0;
 err:
-       q6apm_graph_close(dai_data->graph[dai->id]);
-       dai_data->graph[dai->id] = NULL;
+       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+               q6apm_graph_close(dai_data->graph[dai->id]);
+               dai_data->graph[dai->id] = NULL;
+       }
        return rc;
 }

--------------------->cut<------------------------------

--srini
>  
>  	cfg->direction = substream->stream;


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed
  2025-08-15 15:56 ` Srinivas Kandagatla
@ 2025-08-18 11:47   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-18 11:47 UTC (permalink / raw)
  To: Srinivas Kandagatla, Srinivas Kandagatla, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart,
	linux-sound, linux-arm-msm, linux-kernel
  Cc: stable

On 15/08/2025 17:56, Srinivas Kandagatla wrote:
> Thanks Krzysztof,
> On 8/15/25 12:39 PM, Krzysztof Kozlowski wrote:
>> If earlier opening of source graph fails (e.g. ADSP rejects due to
> 
> I think you are referring to the err patch in prepare.

True I am working on feature relying on that other patch, but the code
here is not really relevant to that other patch, I think.

> 
>> incorrect audioreach topology), the graph is closed and
>> "dai_data->graph[dai->id]" is assigned NULL.  Preparing the DAI for sink
>> graph continues though and next call to q6apm_lpass_dai_prepare()
>> receives dai_data->graph[dai->id]=NULL leading to NULL pointer
>> exception:
>>
>>   qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>   qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>   q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: fail to start APM port 78
>>   q6apm-lpass-dais 30000000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC: error at snd_soc_pcm_dai_prepare on TX_CODEC_DMA_TX_3: -22
>>   Unable to handle kernel NULL pointer dereference at virtual address 00000000000000a8
>>   ...
>>   Call trace:
>>    q6apm_graph_media_format_pcm+0x48/0x120 (P)
>>    q6apm_lpass_dai_prepare+0x110/0x1b4
>>    snd_soc_pcm_dai_prepare+0x74/0x108
>>    __soc_pcm_prepare+0x44/0x160
>>    dpcm_be_dai_prepare+0x124/0x1c0
>>
>> Fixes: 30ad723b93ad ("ASoC: qdsp6: audioreach: add q6apm lpass dai support")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
>> index f90628d9b90e..7520e6f024c3 100644
>> --- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
>> +++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
>> @@ -191,6 +191,12 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
>>  			return rc;
>>  		}
>>  		dai_data->graph[graph_id] = graph;
>> +	} else if (!dai_data->graph[dai->id]) {
>> +		/*
>> +		 * Loading source graph failed before, so abort loading the sink
>> +		 * as well.
>> +		 */
>> +		return -EINVAL;
>>  	}
> I guess this is the capture graph that is triggering the error, normally
> we do not open/close the capture graph in prepare, we do
> stop/prepare/start for capture graphs and handle open close in
> startup/shutdown.
> 
> Can you try this change and see if it fixes the issue, as prepare could
> be called multiple times and your patch will not give chance for trying
> new parameters incase the failure was due to unsupported params.


Yes, this works.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed
  2025-08-15 11:39 [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed Krzysztof Kozlowski
  2025-08-15 15:56 ` Srinivas Kandagatla
@ 2025-09-04 16:51 ` Mark Brown
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Brown @ 2025-09-04 16:51 UTC (permalink / raw)
  To: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	Pierre-Louis Bossart, linux-sound, linux-arm-msm, linux-kernel,
	Srinivas Kandagatla, Krzysztof Kozlowski
  Cc: stable

On Fri, 15 Aug 2025 13:39:16 +0200, Krzysztof Kozlowski wrote:
> If earlier opening of source graph fails (e.g. ADSP rejects due to
> incorrect audioreach topology), the graph is closed and
> "dai_data->graph[dai->id]" is assigned NULL.  Preparing the DAI for sink
> graph continues though and next call to q6apm_lpass_dai_prepare()
> receives dai_data->graph[dai->id]=NULL leading to NULL pointer
> exception:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed
      commit: 68f27f7c7708183e7873c585ded2f1b057ac5b97

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-04 16:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 11:39 [PATCH] ASoC: qcom: q6apm-lpass-dais: Fix NULL pointer dereference if source graph failed Krzysztof Kozlowski
2025-08-15 15:56 ` Srinivas Kandagatla
2025-08-18 11:47   ` Krzysztof Kozlowski
2025-09-04 16:51 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).