[PATCH 0/2] ASoC: qcom: qdsp6: bound the compress ring by the DMA buffer

[PATCH 0/2] ASoC: qcom: qdsp6: bound the compress ring by the DMA buffer

[Julius Bairaktaris]

q6apm-dai and q6asm-dai allocate a fixed 8 MiB DMA buffer when a
compress stream opens and take the ring geometry from
SNDRV_COMPRESS_SET_PARAMS without comparing it to that buffer. Their
copy callbacks place each write() at the stream position modulo the
ring size, so a user who can open the compress node and asks for a
larger ring makes write() copy data of their choice past the buffer.
Each patch rejects such a ring in set_params.

I used Claude (Anthropic) to help write these patches.

Julius Bairaktaris (2):
  ASoC: qcom: q6apm-dai: reject compress rings larger than the DMA
    buffer
  ASoC: qcom: q6asm-dai: reject compress rings larger than the DMA
    buffer

 sound/soc/qcom/qdsp6/q6apm-dai.c | 3 +++
 sound/soc/qcom/qdsp6/q6asm-dai.c | 3 +++
 2 files changed, 6 insertions(+)


base-commit: 20f1752b9241d300b42cd45a07e22d063bccba45
-- 
2.53.0

[PATCH 1/2] ASoC: qcom: q6apm-dai: reject compress rings larger than the DMA buffer

[Julius Bairaktaris]

q6apm_dai_compr_open() allocates a fixed 8 MiB DMA buffer, but
q6apm_dai_compr_set_params() takes the ring size from the fragment size
and count that userspace passes to SNDRV_COMPRESS_SET_PARAMS without
checking it against that buffer. The compress core checks the geometry
against the runtime buffer only for drivers without a copy callback.
q6apm_compr_copy() places each write() at the stream position modulo the
ring size, so a ring larger than the DMA buffer lets write() copy user
data past its end, and after SNDRV_COMPRESS_NEXT_TRACK the copy starts at
the next multiple of the fragment size.

Reject a ring larger than the DMA buffer.

Fixes: c317d148a2b0 ("ASoC: q6dsp: q6apm-dai: Add mmap and copy compress DAI callbacks")
Cc: stable@vger.kernel.org
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 sound/soc/qcom/qdsp6/q6apm-dai.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 58beaf906b9ff..80b837992b429 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -990,6 +990,9 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
 	if (!pdata)
 		return -EINVAL;
 
+	if (runtime->buffer_size > prtd->dma_buffer.bytes)
+		return -EINVAL;
+
 	prtd->periods = runtime->fragments;
 	prtd->pcm_count = runtime->fragment_size;
 	prtd->pcm_size = runtime->fragments * runtime->fragment_size;
-- 
2.53.0

[PATCH 2/2] ASoC: qcom: q6asm-dai: reject compress rings larger than the DMA buffer

[Julius Bairaktaris]

q6asm_dai_compr_open() allocates a fixed 8 MiB DMA buffer, but
q6asm_dai_compr_set_params() takes the ring size from the fragment size
and count that userspace passes to SNDRV_COMPRESS_SET_PARAMS without
checking it against that buffer. The compress core checks the geometry
against the runtime buffer only for drivers without a copy callback.
q6asm_compr_copy() places each write() at the stream position modulo the
ring size, so a ring larger than the DMA buffer lets write() copy user
data past its end, and after SNDRV_COMPRESS_NEXT_TRACK the copy starts at
the next multiple of the fragment size.

Reject a ring larger than the DMA buffer.

Fixes: a08cd56a4524 ("ASoC: q6asm-dai: add support to copy callback")
Cc: stable@vger.kernel.org
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 sound/soc/qcom/qdsp6/q6asm-dai.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index 4261c3048c368..1cd99a1d5b516 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -897,6 +897,9 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component,
 		return -EINVAL;
 	}
 
+	if (runtime->buffer_size > prtd->dma_buffer.bytes)
+		return -EINVAL;
+
 	prtd->periods = runtime->fragments;
 	prtd->pcm_count = runtime->fragment_size;
 	prtd->pcm_size = runtime->fragments * runtime->fragment_size;
-- 
2.53.0

Re: [PATCH 1/2] ASoC: qcom: q6apm-dai: reject compress rings larger than the DMA buffer

[Srinivas Kandagatla]

On 11/11/26 8:06 AM, Julius Bairaktaris wrote:
> q6apm_dai_compr_open() allocates a fixed 8 MiB DMA buffer, but
> q6apm_dai_compr_set_params() takes the ring size from the fragment size
> and count that userspace passes to SNDRV_COMPRESS_SET_PARAMS without
> checking it against that buffer. The compress core checks the geometry
> against the runtime buffer only for drivers without a copy callback.
> q6apm_compr_copy() places each write() at the stream position modulo the
> ring size, so a ring larger than the DMA buffer lets write() copy user
> data past its end, and after SNDRV_COMPRESS_NEXT_TRACK the copy starts at
> the next multiple of the fragment size.
> 
> Reject a ring larger than the DMA buffer.
> 
> Fixes: c317d148a2b0 ("ASoC: q6dsp: q6apm-dai: Add mmap and copy compress DAI callbacks")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
> ---

Thanks for sending the patches

both look good to me, but I would recommend adding a dev_err() so that
its visible to user if it fails.

once that is fixed.

Reviewed-by: Srinivas Kandagatla<srinivas.kandagatla@oss.qualcomm.com>

--Srini

>  sound/soc/qcom/qdsp6/q6apm-dai.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index 58beaf906b9ff..80b837992b429 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
> @@ -990,6 +990,9 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
>  	if (!pdata)
>  		return -EINVAL;
>  
> +	if (runtime->buffer_size > prtd->dma_buffer.bytes)
> +		return -EINVAL;
> +
>  	prtd->periods = runtime->fragments;
>  	prtd->pcm_count = runtime->fragment_size;
>  	prtd->pcm_size = runtime->fragments * runtime->fragment_size;