[PATCH] ASoC: amd: acp: Fix possible UAF in acp_dma_open
Smatch report warning as follows:
sound/soc/amd/acp/acp-platform.c:199 acp_dma_open() warn: '&stream->list' not removed from list
If snd_pcm_hw_constraint_integer() fails in acp_dma_open(), stream will be freed, but stream->list will not be removed from adata->stream_list, then list traversal may cause UAF.
Fix by removeing it from adata->stream_list before free().
Fixes: 7929985cfe36 ("ASoC: amd: acp: Initialize list to store acp_stream during pcm_open") Signed-off-by: Gaosheng Cui cuigaosheng1@huawei.com --- sound/soc/amd/acp/acp-platform.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c index 85a81add4ef9..275e0428eec4 100644 --- a/sound/soc/amd/acp/acp-platform.c +++ b/sound/soc/amd/acp/acp-platform.c @@ -196,6 +196,9 @@ static int acp_dma_open(struct snd_soc_component *component, struct snd_pcm_subs ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); if (ret < 0) { dev_err(component->dev, "set integer constraint failed\n"); + spin_lock_irq(&adata->acp_lock); + list_del(&stream->list); + spin_unlock_irq(&adata->acp_lock); kfree(stream); return ret; }
On Thu, Nov 17, 2022 at 02:12:48PM +0800, Gaosheng Cui wrote:
Smatch report warning as follows:
sound/soc/amd/acp/acp-platform.c:199 acp_dma_open() warn: '&stream->list' not removed from list
If snd_pcm_hw_constraint_integer() fails in acp_dma_open(), stream will be freed, but stream->list will not be removed from adata->stream_list, then list traversal may cause UAF.
Is it not better to only add the newly allocated stream to the list once it's fully initialised? Otherwise something could be using a partially initialised item from the list.
Is it not better to only add the newly allocated stream to the list once it's fully initialised? Otherwise something could be using a partially initialised item from the list.
Thanks for taking time to review this patch.
I have made a patch v2 and submit it, fix it by adding the newly allocated stream to the list once it's fully initialised.
On 2022/11/17 19:16, Mark Brown wrote:
On Thu, Nov 17, 2022 at 02:12:48PM +0800, Gaosheng Cui wrote:
Smatch report warning as follows:
sound/soc/amd/acp/acp-platform.c:199 acp_dma_open() warn: '&stream->list' not removed from list
If snd_pcm_hw_constraint_integer() fails in acp_dma_open(), stream will be freed, but stream->list will not be removed from adata->stream_list, then list traversal may cause UAF.
Is it not better to only add the newly allocated stream to the list once it's fully initialised? Otherwise something could be using a partially initialised item from the list.
participants (3)
-
cuigaosheng -
Gaosheng Cui
-
Mark Brown