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) {
?
Jes
--
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
