Re: [PATCH 2/2] ASoC: topology: Fix a small leak in soc_tplg_dapm_graph_elems_load()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/20/2021 7:07 AM, Dan Carpenter wrote:
We have to kfree(routes) on this error path.

Fixes: ff9226224437 ("ASoC: topology: Change allocations to resource managed")
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
  sound/soc/soc-topology.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 6b7a813bc264..5730fcaa7bc6 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1135,8 +1135,10 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
  	 */
  	for (i = 0; i < count; i++) {
  		routes[i] = devm_kzalloc(tplg->dev, sizeof(*routes[i]), GFP_KERNEL);
-		if (!routes[i])
-			return -ENOMEM;
+		if (!routes[i]) {
+			ret = -ENOMEM;
+			goto free_routes;
+		}
  	}
for (i = 0; i < count; i++) {
@@ -1198,6 +1200,7 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
  	 * The memory allocated for each dapm route will be freed
  	 * when it is removed in remove_route().
  	 */
+free_routes:
  	kfree(routes);
return ret;


Yes, that's right, however looking at this function again, I wonder if instead we can just get rid of the routes array and kcalloc call?

Something along those lines (hope that copy paste won't mess it up):



diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 73076d425efb..13db9cfe223f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1104,7 +1104,7 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
 {
        struct snd_soc_dapm_context *dapm = &tplg->comp->dapm;
        struct snd_soc_tplg_dapm_graph_elem *elem;
-       struct snd_soc_dapm_route **routes;
+       struct snd_soc_dapm_route *route;
        int count, i;
        int ret = 0;

@@ -1122,28 +1122,15 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, dev_dbg(tplg->dev, "ASoC: adding %d DAPM routes for index %d\n", count,
                hdr->index);

-       /* allocate memory for pointer to array of dapm routes */
-       routes = kcalloc(count, sizeof(struct snd_soc_dapm_route *),
-                        GFP_KERNEL);
-       if (!routes)
-               return -ENOMEM;
-
-       /*
-        * allocate memory for each dapm route in the array.
-        * This needs to be done individually so that
-        * each route can be freed when it is removed in remove_route().
-        */
        for (i = 0; i < count; i++) {
- routes[i] = devm_kzalloc(tplg->dev, sizeof(*routes[i]), GFP_KERNEL);
-               if (!routes[i])
+               route = devm_kzalloc(tplg->dev, sizeof(*route), GFP_KERNEL);
+               if (!route)
                        return -ENOMEM;
-       }

-       for (i = 0; i < count; i++) {
                elem = (struct snd_soc_tplg_dapm_graph_elem *)tplg->pos;
                tplg->pos += sizeof(struct snd_soc_tplg_dapm_graph_elem);

-               /* validate routes */
+               /* validate route */
                if (strnlen(elem->source, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) ==
                            SNDRV_CTL_ELEM_ID_NAME_MAXLEN) {
                        ret = -EINVAL;
@@ -1160,46 +1147,32 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
                        break;
                }

-               routes[i]->source = elem->source;
-               routes[i]->sink = elem->sink;
+               route->source = elem->source;
+               route->sink = elem->sink;

                /* set to NULL atm for tplg users */
-               routes[i]->connected = NULL;
+               route->connected = NULL;
if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 0)
-                       routes[i]->control = NULL;
+                       route->control = NULL;
                else
-                       routes[i]->control = elem->control;
+                       route->control = elem->control;

                /* add route dobj to dobj_list */
-               routes[i]->dobj.type = SND_SOC_DOBJ_GRAPH;
-               routes[i]->dobj.ops = tplg->ops;
-               routes[i]->dobj.index = tplg->index;
-               list_add(&routes[i]->dobj.list, &tplg->comp->dobj_list);
+               route->dobj.type = SND_SOC_DOBJ_GRAPH;
+               route->dobj.ops = tplg->ops;
+               route->dobj.index = tplg->index;
+               list_add(&route->dobj.list, &tplg->comp->dobj_list);

-               ret = soc_tplg_add_route(tplg, routes[i]);
+               ret = soc_tplg_add_route(tplg, route);
                if (ret < 0) {
dev_err(tplg->dev, "ASoC: topology: add_route failed: %d\n", ret);
-                       /*
-                        * this route was added to the list, it will
-                        * be freed in remove_route() so increment the
-                        * counter to skip it in the error handling
-                        * below.
-                        */
-                       i++;
                        break;
                }

                /* add route, but keep going if some fail */
-               snd_soc_dapm_add_routes(dapm, routes[i], 1);
+               snd_soc_dapm_add_routes(dapm, route, 1);
        }

-       /*
-        * free pointer to array of dapm routes as this is no longer needed.
-        * The memory allocated for each dapm route will be freed
-        * when it is removed in remove_route().
-        */
-       kfree(routes);
-
        return ret;
 }





[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux