Re: [PATCH] Free map to avoid resource leak issues

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

 





On 06/09/2018 01:00 AM, Jes Sorensen wrote:
On 06/08/2018 04:16 AM, Guoqing Jiang wrote:
There are some places which didn't free map as
discovered by coverity.

CID 289661 (#1 of 1): Resource leak (RESOURCE_LEAK)12. leaked_storage: Variable mapl going out of scope leaks the storage it points to.
CID 289619 (#3 of 3): Resource leak (RESOURCE_LEAK)63. leaked_storage: Variable map going out of scope leaks the storage it points to.
CID 289618 (#1 of 1): Resource leak (RESOURCE_LEAK)26. leaked_storage: Variable map going out of scope leaks the storage it points to.
CID 289607 (#1 of 1): Resource leak (RESOURCE_LEAK)41. leaked_storage: Variable map going out of scope leaks the storage it points to.

And map_unlock is always called with map_lock,
if we don't call map_remove before map_unlock,
then the memory (allocated by  map_lock -> map_read
-> map_add -> xmalloc) could be leaked. So we
need to free it in map_unlock as well.

Signed-off-by: Guoqing Jiang <gqjiang@xxxxxxxx>
---
  Assemble.c    | 2 +-
  Detail.c      | 2 ++
  Incremental.c | 2 ++
  mapfile.c     | 2 ++
  mdadm.c       | 1 +
  5 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Assemble.c b/Assemble.c
index e83d550b2c7b..1cce780986ee 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1849,8 +1849,8 @@ try_again:
  	if (rv == 1 && !pre_exist)
  		ioctl(mdfd, STOP_ARRAY, NULL);
  	free(devices);
-	map_unlock(&map);
  out:
+	map_unlock(&map);
  	if (rv == 0) {
  		wait_for(chosen_name, mdfd);
  		close(mdfd);
diff --git a/Detail.c b/Detail.c
index 860241ce2017..b3e857a7e2c9 100644
--- a/Detail.c
+++ b/Detail.c
@@ -263,6 +263,7 @@ int Detail(char *dev, struct context *c)
if (st->ss->export_detail_super)
  				st->ss->export_detail_super(st);
+			map_free(map);
  		} else {
  			struct map_ent *mp, *map = NULL;
  			char nbuf[64];
@@ -277,6 +278,7 @@ int Detail(char *dev, struct context *c)
  				print_escape(mp->path+8);
  				putchar('\n');
  			}
+			map_free(map);
  		}
  		if (sra) {
  			struct mdinfo *mdi;
diff --git a/Incremental.c b/Incremental.c
index 0beab163e642..94b0a047ca66 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -1413,6 +1413,7 @@ restart:
  			sysfs_free(sra);
  		}
  	}
+	map_free(mapl);
  	return rv;
  }
@@ -1663,6 +1664,7 @@ static int Incremental_container(struct supertype *st, char *devname,
  			close(sfd);
  	}
  	domain_free(domains);
+	map_free(map);
  	return 0;
  }
diff --git a/mapfile.c b/mapfile.c
index f3c8191e80b7..a50255632d28 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -143,6 +143,8 @@ void map_unlock(struct map_ent **melp)
  		unlink(mapname[2]);
  		fclose(lf);
  	}
+	if (*melp)
+		map_free(*melp);
  	lf = NULL;
  }
diff --git a/mdadm.c b/mdadm.c
index 5afe4155d701..9f2c67a5b0c1 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1887,6 +1887,7 @@ static int misc_scan(char devmode, struct context *c)
  			put_md_name(name);
  		}
  	}
+	map_free(map);
  	free_mdstat(ms);
  	return rv;
  }

Shouldn't the map_free() in mdadm.c be inside the loop:

		for (e = ms; e; e = e->next) {

?

Right, my mistake and thanks for review.

Regards,
Guoqing

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux