diff mbox

btrfs-progs: fix memory leaks in error path

Message ID 1440126612-53381-1-git-send-email-bhlee.kernel@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Byongho Lee Aug. 21, 2015, 3:10 a.m. UTC 
This patch includes below fixes in error path:
1. fix memory leaks if realloc() fails
2. add missing call free_history() before return error in scrub_read_file()

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 btrfs-list.c |  8 ++++++++
 cmds-scrub.c | 18 ++++++++++++++----
 qgroup.c     |  8 ++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

Comments

Zhaolei Aug. 21, 2015, 3:50 a.m. UTC | #1 
Hi, Byongho Lee

> -----Original Message-----
> From: linux-btrfs-owner@vger.kernel.org
> [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of Byongho Lee
> Sent: Friday, August 21, 2015 11:10 AM
> To: linux-btrfs@vger.kernel.org
> Subject: [PATCH] btrfs-progs: fix memory leaks in error path
> 
> This patch includes below fixes in error path:
> 1. fix memory leaks if realloc() fails
> 2. add missing call free_history() before return error in scrub_read_file()
> 
> Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
> ---
>  btrfs-list.c |  8 ++++++++
>  cmds-scrub.c | 18 ++++++++++++++----
>  qgroup.c     |  8 ++++++++


Similar problem in cmds-send.c:
cmds-send.c:    s->clone_sources = realloc(s->clone_sources,

Thanks
Zhaolei

>  3 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/btrfs-list.c b/btrfs-list.c index 875a89dc4ef0..d54de61aec01 100644
> --- a/btrfs-list.c
> +++ b/btrfs-list.c
> @@ -254,11 +254,15 @@ static int btrfs_list_setup_comparer(struct
> btrfs_list_comparer_set **comp_set,
>  	BUG_ON(set->ncomps > set->total);
> 
>  	if (set->ncomps == set->total) {
> +		void *tmp;
> +
>  		size = set->total + BTRFS_LIST_NCOMPS_INCREASE;
>  		size = sizeof(*set) + size * sizeof(struct btrfs_list_comparer);
> +		tmp = set;
>  		set = realloc(set, size);
>  		if (!set) {
>  			fprintf(stderr, "memory allocation failed\n");
> +			free(tmp);
>  			exit(1);
>  		}
> 
> @@ -1232,11 +1236,15 @@ int btrfs_list_setup_filter(struct
> btrfs_list_filter_set **filter_set,
>  	BUG_ON(set->nfilters > set->total);
> 
>  	if (set->nfilters == set->total) {
> +		void *tmp;
> +
>  		size = set->total + BTRFS_LIST_NFILTERS_INCREASE;
>  		size = sizeof(*set) + size * sizeof(struct btrfs_list_filter);
> +		tmp = set;
>  		set = realloc(set, size);
>  		if (!set) {
>  			fprintf(stderr, "memory allocation failed\n");
> +			free(tmp);
>  			exit(1);
>  		}
> 
> diff --git a/cmds-scrub.c b/cmds-scrub.c index 5a85dc473c94..91cf67841849
> 100644
> --- a/cmds-scrub.c
> +++ b/cmds-scrub.c
> @@ -502,12 +502,16 @@ again:
>  		}
>  		return p;
>  	}
> -	if (avail == -1)
> +	if (avail == -1) {
> +		free_history(p);
>  		return ERR_PTR(-errno);
> +	}
>  	avail += old_avail;
> 
>  	i = 0;
>  	while (i < avail) {
> +		void *tmp;
> +
>  		switch (state) {
>  		case 0: /* start of file */
>  			ret = scrub_kvread(&i,
> @@ -534,11 +538,17 @@ again:
>  				continue;
>  			}
>  			++curr;
> +			tmp = p;
>  			p = realloc(p, (curr + 2) * sizeof(*p));
> -			if (p)
> -				p[curr] = malloc(sizeof(**p));
> -			if (!p || !p[curr])
> +			if (!p) {
> +				free_history(tmp);
>  				return ERR_PTR(-errno);
> +			}
> +			p[curr] = malloc(sizeof(**p));
> +			if (!p[curr]) {
> +				free_history(p);
> +				return ERR_PTR(-errno);
> +			}
>  			memset(p[curr], 0, sizeof(**p));
>  			p[curr + 1] = NULL;
>  			++state;
> diff --git a/qgroup.c b/qgroup.c
> index dc04b033b145..327abd645f16 100644
> --- a/qgroup.c
> +++ b/qgroup.c
> @@ -465,12 +465,16 @@ int btrfs_qgroup_setup_comparer(struct
> btrfs_qgroup_comparer_set  **comp_set,
>  	BUG_ON(set->ncomps > set->total);
> 
>  	if (set->ncomps == set->total) {
> +		void *tmp;
> +
>  		size = set->total + BTRFS_QGROUP_NCOMPS_INCREASE;
>  		size = sizeof(*set) +
>  		       size * sizeof(struct btrfs_qgroup_comparer);
> +		tmp = set;
>  		set = realloc(set, size);
>  		if (!set) {
>  			fprintf(stderr, "memory allocation failed\n");
> +			free(tmp);
>  			exit(1);
>  		}
> 
> @@ -836,12 +840,16 @@ int btrfs_qgroup_setup_filter(struct
> btrfs_qgroup_filter_set **filter_set,
>  	BUG_ON(set->nfilters > set->total);
> 
>  	if (set->nfilters == set->total) {
> +		void *tmp;
> +
>  		size = set->total + BTRFS_QGROUP_NFILTERS_INCREASE;
>  		size = sizeof(*set) + size * sizeof(struct btrfs_qgroup_filter);
> 
> +		tmp = set;
>  		set = realloc(set, size);
>  		if (!set) {
>  			fprintf(stderr, "memory allocation failed\n");
> +			free(tmp);
>  			exit(1);
>  		}
>  		memset(&set->filters[set->total], 0,
> --
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Byongho Lee Aug. 21, 2015, 5:32 a.m. UTC | #2 
Zhao Lei writes:

> Hi, Byongho Lee
>
>> -----Original Message-----
>> From: linux-btrfs-owner@vger.kernel.org
>> [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of Byongho Lee
>> Sent: Friday, August 21, 2015 11:10 AM
>> To: linux-btrfs@vger.kernel.org
>> Subject: [PATCH] btrfs-progs: fix memory leaks in error path
>> 
>> This patch includes below fixes in error path:
>> 1. fix memory leaks if realloc() fails
>> 2. add missing call free_history() before return error in scrub_read_file()
>> 
>> Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
>> ---
>>  btrfs-list.c |  8 ++++++++
>>  cmds-scrub.c | 18 ++++++++++++++----
>>  qgroup.c     |  8 ++++++++
>
>
> Similar problem in cmds-send.c:
> cmds-send.c:    s->clone_sources = realloc(s->clone_sources,
>

Thank you for feedback.

You're right, I missed that point.
I'll prepare v2 patch.

Regards,
Lee
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Aug. 25, 2015, 3:36 p.m. UTC | #3 
On Fri, Aug 21, 2015 at 12:10:12PM +0900, Byongho Lee wrote:
> This patch includes below fixes in error path:
> 1. fix memory leaks if realloc() fails
> 2. add missing call free_history() before return error in scrub_read_file()
> 
> Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/btrfs-list.c b/btrfs-list.c
index 875a89dc4ef0..d54de61aec01 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -254,11 +254,15 @@  static int btrfs_list_setup_comparer(struct btrfs_list_comparer_set **comp_set,
 	BUG_ON(set->ncomps > set->total);
 
 	if (set->ncomps == set->total) {
+		void *tmp;
+
 		size = set->total + BTRFS_LIST_NCOMPS_INCREASE;
 		size = sizeof(*set) + size * sizeof(struct btrfs_list_comparer);
+		tmp = set;
 		set = realloc(set, size);
 		if (!set) {
 			fprintf(stderr, "memory allocation failed\n");
+			free(tmp);
 			exit(1);
 		}
 
@@ -1232,11 +1236,15 @@  int btrfs_list_setup_filter(struct btrfs_list_filter_set **filter_set,
 	BUG_ON(set->nfilters > set->total);
 
 	if (set->nfilters == set->total) {
+		void *tmp;
+
 		size = set->total + BTRFS_LIST_NFILTERS_INCREASE;
 		size = sizeof(*set) + size * sizeof(struct btrfs_list_filter);
+		tmp = set;
 		set = realloc(set, size);
 		if (!set) {
 			fprintf(stderr, "memory allocation failed\n");
+			free(tmp);
 			exit(1);
 		}
 
diff --git a/cmds-scrub.c b/cmds-scrub.c
index 5a85dc473c94..91cf67841849 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -502,12 +502,16 @@  again:
 		}
 		return p;
 	}
-	if (avail == -1)
+	if (avail == -1) {
+		free_history(p);
 		return ERR_PTR(-errno);
+	}
 	avail += old_avail;
 
 	i = 0;
 	while (i < avail) {
+		void *tmp;
+
 		switch (state) {
 		case 0: /* start of file */
 			ret = scrub_kvread(&i,
@@ -534,11 +538,17 @@  again:
 				continue;
 			}
 			++curr;
+			tmp = p;
 			p = realloc(p, (curr + 2) * sizeof(*p));
-			if (p)
-				p[curr] = malloc(sizeof(**p));
-			if (!p || !p[curr])
+			if (!p) {
+				free_history(tmp);
 				return ERR_PTR(-errno);
+			}
+			p[curr] = malloc(sizeof(**p));
+			if (!p[curr]) {
+				free_history(p);
+				return ERR_PTR(-errno);
+			}
 			memset(p[curr], 0, sizeof(**p));
 			p[curr + 1] = NULL;
 			++state;
diff --git a/qgroup.c b/qgroup.c
index dc04b033b145..327abd645f16 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -465,12 +465,16 @@  int btrfs_qgroup_setup_comparer(struct btrfs_qgroup_comparer_set  **comp_set,
 	BUG_ON(set->ncomps > set->total);
 
 	if (set->ncomps == set->total) {
+		void *tmp;
+
 		size = set->total + BTRFS_QGROUP_NCOMPS_INCREASE;
 		size = sizeof(*set) +
 		       size * sizeof(struct btrfs_qgroup_comparer);
+		tmp = set;
 		set = realloc(set, size);
 		if (!set) {
 			fprintf(stderr, "memory allocation failed\n");
+			free(tmp);
 			exit(1);
 		}
 
@@ -836,12 +840,16 @@  int btrfs_qgroup_setup_filter(struct btrfs_qgroup_filter_set **filter_set,
 	BUG_ON(set->nfilters > set->total);
 
 	if (set->nfilters == set->total) {
+		void *tmp;
+
 		size = set->total + BTRFS_QGROUP_NFILTERS_INCREASE;
 		size = sizeof(*set) + size * sizeof(struct btrfs_qgroup_filter);
 
+		tmp = set;
 		set = realloc(set, size);
 		if (!set) {
 			fprintf(stderr, "memory allocation failed\n");
+			free(tmp);
 			exit(1);
 		}
 		memset(&set->filters[set->total], 0,