[alsa-devel] [PATCH 1/2] ASoC: qdsp6: q6asm-dai: Off by one in of_q6asm_parse_dai_data()
The q6asm_fe_dais[] array has MAX_SESSIONS (8) elements so the > comparison should be >= or we access one element beyond the end of the array.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- sound/soc/qcom/qdsp6/q6asm-dai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 5b986b74dd36..9d738b4c1e05 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -874,7 +874,7 @@ static int of_q6asm_parse_dai_data(struct device *dev,
for_each_child_of_node(dev->of_node, node) { ret = of_property_read_u32(node, "reg", &id); - if (ret || id > MAX_SESSIONS || id < 0) { + if (ret || id >= MAX_SESSIONS || id < 0) { dev_err(dev, "valid dai id not found:%d\n", ret); continue; }
The q6asm_audio_client_alloc() doesn't return NULL, it returns error pointers.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- sound/soc/qcom/qdsp6/q6asm-dai.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 9d738b4c1e05..3407e51b8861 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -570,10 +570,11 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) prtd->audio_client = q6asm_audio_client_alloc(dev, (q6asm_cb)compress_event_handler, prtd, stream_id, LEGACY_PCM_MODE); - if (!prtd->audio_client) { + if (IS_ERR(prtd->audio_client)) { dev_err(dev, "Could not allocate memory\n"); + ret = PTR_ERR(prtd->audio_client); kfree(prtd); - return -ENOMEM; + return ret; }
size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE *
On 21/12/2018 09:05, Dan Carpenter wrote:
The q6asm_audio_client_alloc() doesn't return NULL, it returns error pointers.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
Thanks for the patch!
Acked-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
The patch
ASoC: qdsp6: q6asm-dai: Fix a NULL vs IS_ERR() bug
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 168f6a0b0d2a9c662a6b5d839385dcce70e8c518 Mon Sep 17 00:00:00 2001
From: Dan Carpenter dan.carpenter@oracle.com Date: Fri, 21 Dec 2018 12:05:16 +0300 Subject: [PATCH] ASoC: qdsp6: q6asm-dai: Fix a NULL vs IS_ERR() bug
The q6asm_audio_client_alloc() doesn't return NULL, it returns error pointers.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/qcom/qdsp6/q6asm-dai.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 9d738b4c1e05..3407e51b8861 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -570,10 +570,11 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) prtd->audio_client = q6asm_audio_client_alloc(dev, (q6asm_cb)compress_event_handler, prtd, stream_id, LEGACY_PCM_MODE); - if (!prtd->audio_client) { + if (IS_ERR(prtd->audio_client)) { dev_err(dev, "Could not allocate memory\n"); + ret = PTR_ERR(prtd->audio_client); kfree(prtd); - return -ENOMEM; + return ret; }
size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE *
We can't return directly if snd_dma_alloc_pages() fails; we first need to free prtd->audio_client and prtd.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- sound/soc/qcom/qdsp6/q6asm-dai.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 3407e51b8861..548eb4fa2da6 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -573,8 +573,7 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) if (IS_ERR(prtd->audio_client)) { dev_err(dev, "Could not allocate memory\n"); ret = PTR_ERR(prtd->audio_client); - kfree(prtd); - return ret; + goto free_prtd; }
size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE * @@ -583,7 +582,7 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) &prtd->dma_buffer); if (ret) { dev_err(dev, "Cannot allocate buffer(s)\n"); - return ret; + goto free_client; }
if (pdata->sid < 0) @@ -596,6 +595,13 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) runtime->private_data = prtd;
return 0; + +free_client: + q6asm_audio_client_free(prtd->audio_client); +free_prtd: + kfree(prtd); + + return ret; }
static int q6asm_dai_compr_free(struct snd_compr_stream *stream)
On 21/12/2018 09:06, Dan Carpenter wrote:
We can't return directly if snd_dma_alloc_pages() fails; we first need to free prtd->audio_client and prtd.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
Thanks for the patch! I was due to send something similar!
Acked-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
The patch
ASoC: qdsp6: q6asm-dai: Fix a small memory leak
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 686a6dace3bd783ef9f747e9c2cff68038be0e3a Mon Sep 17 00:00:00 2001
From: Dan Carpenter dan.carpenter@oracle.com Date: Fri, 21 Dec 2018 12:06:10 +0300 Subject: [PATCH] ASoC: qdsp6: q6asm-dai: Fix a small memory leak
We can't return directly if snd_dma_alloc_pages() fails; we first need to free prtd->audio_client and prtd.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/qcom/qdsp6/q6asm-dai.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 3407e51b8861..548eb4fa2da6 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -573,8 +573,7 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) if (IS_ERR(prtd->audio_client)) { dev_err(dev, "Could not allocate memory\n"); ret = PTR_ERR(prtd->audio_client); - kfree(prtd); - return ret; + goto free_prtd; }
size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE * @@ -583,7 +582,7 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) &prtd->dma_buffer); if (ret) { dev_err(dev, "Cannot allocate buffer(s)\n"); - return ret; + goto free_client; }
if (pdata->sid < 0) @@ -596,6 +595,13 @@ static int q6asm_dai_compr_open(struct snd_compr_stream *stream) runtime->private_data = prtd;
return 0; + +free_client: + q6asm_audio_client_free(prtd->audio_client); +free_prtd: + kfree(prtd); + + return ret; }
static int q6asm_dai_compr_free(struct snd_compr_stream *stream)
The problem is seen in the q6asm_dai_compr_set_params() function:
ret = q6asm_map_memory_regions(dir, prtd->audio_client, prtd->phys, (prtd->pcm_size / prtd->periods), ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ prtd->periods);
In this code prtd->pcm_size is the buffer_size and prtd->periods comes from params->buffer.fragments. If we allow the number of fragments to be zero then it results in a divide by zero bug. One possible fix would be to use prtd->pcm_count directly instead of using the division to re-calculate it. But I decided that it doesn't really make sense to allow zero fragments.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- I am not very sure of this patch. Please review it extra carefully because it is an API change.
sound/core/compress_offload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index a5b09e75e787..f7d2b373da0a 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -541,7 +541,8 @@ static int snd_compress_check_input(struct snd_compr_params *params) { /* first let's check the buffer parameter's */ if (params->buffer.fragment_size == 0 || - params->buffer.fragments > INT_MAX / params->buffer.fragment_size) + params->buffer.fragments > INT_MAX / params->buffer.fragment_size || + params->buffer.fragments == 0) return -EINVAL;
/* now codec parameters */
The patch
ALSA: compress: prevent potential divide by zero bugs
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From e9d960271d89c0cabf094123e2ddc94705de2a31 Mon Sep 17 00:00:00 2001
From: Dan Carpenter dan.carpenter@oracle.com Date: Fri, 21 Dec 2018 12:06:58 +0300 Subject: [PATCH] ALSA: compress: prevent potential divide by zero bugs
The problem is seen in the q6asm_dai_compr_set_params() function:
ret = q6asm_map_memory_regions(dir, prtd->audio_client, prtd->phys, (prtd->pcm_size / prtd->periods), ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ prtd->periods);
In this code prtd->pcm_size is the buffer_size and prtd->periods comes from params->buffer.fragments. If we allow the number of fragments to be zero then it results in a divide by zero bug. One possible fix would be to use prtd->pcm_count directly instead of using the division to re-calculate it. But I decided that it doesn't really make sense to allow zero fragments.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/core/compress_offload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index a5b09e75e787..f7d2b373da0a 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -541,7 +541,8 @@ static int snd_compress_check_input(struct snd_compr_params *params) { /* first let's check the buffer parameter's */ if (params->buffer.fragment_size == 0 || - params->buffer.fragments > INT_MAX / params->buffer.fragment_size) + params->buffer.fragments > INT_MAX / params->buffer.fragment_size || + params->buffer.fragments == 0) return -EINVAL;
/* now codec parameters */
On 21/12/2018 09:04, Dan Carpenter wrote:
The q6asm_fe_dais[] array has MAX_SESSIONS (8) elements so the > comparison should be >= or we access one element beyond the end of the array.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
Thanks for the patch!
Acked-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
The patch
ASoC: qdsp6: q6asm-dai: Off by one in of_q6asm_parse_dai_data()
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 6f10aff97f5d2c9a26a4deff1482f845841b7307 Mon Sep 17 00:00:00 2001
From: Dan Carpenter dan.carpenter@oracle.com Date: Fri, 21 Dec 2018 12:04:42 +0300 Subject: [PATCH] ASoC: qdsp6: q6asm-dai: Off by one in of_q6asm_parse_dai_data()
The q6asm_fe_dais[] array has MAX_SESSIONS (8) elements so the > comparison should be >= or we access one element beyond the end of the array.
Fixes: 22930c79ac5c ("ASoC: qdsp6: q6asm-dai: Add support to compress offload") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/qcom/qdsp6/q6asm-dai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 5b986b74dd36..9d738b4c1e05 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -874,7 +874,7 @@ static int of_q6asm_parse_dai_data(struct device *dev,
for_each_child_of_node(dev->of_node, node) { ret = of_property_read_u32(node, "reg", &id); - if (ret || id > MAX_SESSIONS || id < 0) { + if (ret || id >= MAX_SESSIONS || id < 0) { dev_err(dev, "valid dai id not found:%d\n", ret); continue; }
participants (3)
-
Dan Carpenter -
Mark Brown -
Srinivas Kandagatla