| Message ID | 20221117061248.3018292-1-cuigaosheng1@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: amd: acp: Fix possible UAF in acp_dma_open | expand |
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.
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; }
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(+)