| Message ID | 20231116132849.1534-1-kamil.duljas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: SOF: topology: Fix mem leak in sof_dai_load() | expand |
On Thu, 2023-11-16 at 14:28 +0100, Kamil Duljas wrote: > The function has multiple return points at which it is not released > previously allocated memory. > > Signed-off-by: Kamil Duljas <kamil.duljas@gmail.com> > --- > sound/soc/sof/topology.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c > index a3a3af252259..ef8f8991f025 100644 > --- a/sound/soc/sof/topology.c > +++ b/sound/soc/sof/topology.c > @@ -1736,8 +1736,10 @@ static int sof_dai_load(struct > snd_soc_component *scomp, int index, > /* perform pcm set op */ > if (ipc_pcm_ops && ipc_pcm_ops->pcm_setup) { > ret = ipc_pcm_ops->pcm_setup(sdev, spcm); > - if (ret < 0) > + if (ret < 0) { > + kfree(spcm); > return ret; > + } Thanks for the patch, Kamil. I think just the change above should suffice to fix the memory leak in case of error. For all the other error paths below, snd_soc_tplg_component_remove() should be able to handle freeing the spcm and the page tables. Thanks, Ranjani
Hello Ranjani, You right. My mistake. I will prepare patch v2 Kamil czw., 16 lis 2023 o 19:57 Sridharan, Ranjani <ranjani.sridharan@intel.com> napisał(a): > On Thu, 2023-11-16 at 14:28 +0100, Kamil Duljas wrote: > > The function has multiple return points at which it is not released > > previously allocated memory. > > > > Signed-off-by: Kamil Duljas <kamil.duljas@gmail.com> > > --- > > sound/soc/sof/topology.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c > > index a3a3af252259..ef8f8991f025 100644 > > --- a/sound/soc/sof/topology.c > > +++ b/sound/soc/sof/topology.c > > @@ -1736,8 +1736,10 @@ static int sof_dai_load(struct > > snd_soc_component *scomp, int index, > > /* perform pcm set op */ > > if (ipc_pcm_ops && ipc_pcm_ops->pcm_setup) { > > ret = ipc_pcm_ops->pcm_setup(sdev, spcm); > > - if (ret < 0) > > + if (ret < 0) { > > + kfree(spcm); > > return ret; > > + } > > Thanks for the patch, Kamil. I think just the change above should > suffice to fix the memory leak in case of error. For all the other > error paths below, snd_soc_tplg_component_remove() should be able to > handle freeing the spcm and the page tables. > > Thanks, > Ranjani >
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index a3a3af252259..ef8f8991f025 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1736,8 +1736,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, /* perform pcm set op */ if (ipc_pcm_ops && ipc_pcm_ops->pcm_setup) { ret = ipc_pcm_ops->pcm_setup(sdev, spcm); - if (ret < 0) + if (ret < 0) { + kfree(spcm); return ret; + } } dai_drv->dobj.private = spcm; @@ -1747,6 +1749,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, ARRAY_SIZE(stream_tokens), private->array, le32_to_cpu(private->size)); if (ret) { + kfree(dai_drv->dobj.private); dev_err(scomp->dev, "error: parse stream tokens failed %d\n", le32_to_cpu(private->size)); return ret; @@ -1764,9 +1767,9 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, sdev->dev, PAGE_SIZE, &spcm->stream[stream].page_table); if (ret < 0) { + kfree(dai_drv->dobj.private); dev_err(scomp->dev, "error: can't alloc page table for %s %d\n", caps->name, ret); - return ret; } @@ -1782,9 +1785,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, stream = SNDRV_PCM_STREAM_CAPTURE; /* do we need to allocate capture PCM DMA pages */ - if (!spcm->pcm.capture) + if (!spcm->pcm.capture) { + kfree(dai_drv->dobj.private); return ret; - + } caps = &spcm->pcm.caps[stream]; /* allocate capture page table buffer */ @@ -1810,7 +1814,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, free_playback_tables: if (spcm->pcm.playback) snd_dma_free_pages(&spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].page_table); - + kfree(dai_drv->dobj.private); return ret; }
The function has multiple return points at which it is not released previously allocated memory. Signed-off-by: Kamil Duljas <kamil.duljas@gmail.com> --- sound/soc/sof/topology.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)