[PATCH 00/11] Memory/resource leaks and unchecked return fixes

[PATCH 00/11] Memory/resource leaks and unchecked return fixes

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

This is another pile of patches to fixup memory leaks and buffer
overflows found in the coverity run. 

Cheers,
Jes


Jes Sorensen (11):
  Fix memory leaks in reshape_array()
  Fix memory leak
  Fix memory leak
  Fix memory leak of 'st3' in array_try_spare()
  partition_try_spare() use closedir() to release DIR * returned by
    opendir()
  Fix memory leak
  Add missing return in case of trying to grow sub-array
  Avoid memory leak
  policy_add(): Add missing va_end()
  Write_rules(): Avoid stack corruption if using extremely long udev
    pathname
  mdmon(): Error out if failing to connect to victim monitor

 Assemble.c    |    1 +
 Detail.c      |    1 +
 Grow.c        |   16 ++++++++++++++--
 Incremental.c |    6 +++++-
 mdmon.c       |    8 +++++++-
 policy.c      |    4 +++-
 super-gpt.c   |    4 +++-
 super-intel.c |    1 +
 8 files changed, 35 insertions(+), 6 deletions(-)

-- 
1.7.6.4

[PATCH 01/11] Fix memory leaks in reshape_array()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Grow.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Grow.c b/Grow.c
index 08581d2..05350dd 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1882,8 +1882,8 @@ static int reshape_array(char *container, int fd, char *devname,
 	struct mddev_dev *dv;
 	int added_disks;
 
-	int *fdlist;
-	unsigned long long *offsets;
+	int *fdlist = NULL;
+	unsigned long long *offsets = NULL;
 	int d;
 	int nrdisks;
 	int err;
@@ -2339,6 +2339,9 @@ started:
 		abort_reshape(sra);
 		goto release;
 	default:
+		free(fdlist);
+		free(offsets);
+		sysfs_free(sra);
 		return 0;
 	case 0:
 		map_fork();
@@ -2366,6 +2369,9 @@ started:
 			d - odisks, fdlist+odisks,
 			offsets+odisks);
 
+	free(fdlist);
+	free(offsets);
+
 	if (backup_file && done)
 		unlink(backup_file);
 	if (!done) {
@@ -2381,6 +2387,7 @@ started:
 		/* no need to wait for the reshape to finish as
 		 * there is nothing more to do.
 		 */
+		sysfs_free(sra);
 		exit(0);
 	}
 	wait_reshape(sra);
@@ -2445,17 +2452,21 @@ started:
 			st->update_tail = NULL;
 	}
 out:
+	sysfs_free(sra);
 	if (forked)
 		return 0;
 	unfreeze(st);
 	exit(0);
 
 release:
+	free(fdlist);
+	free(offsets);
 	if (orig_level != UnSet && sra) {
 		c = map_num(pers, orig_level);
 		if (c && sysfs_set_str(sra, NULL, "level", c) == 0)
 			fprintf(stderr, Name ": aborting level change\n");
 	}
+	sysfs_free(sra);
 	if (!forked)
 		unfreeze(st);
 	return 1;
-- 
1.7.6.4

[PATCH 02/11] Fix memory leak

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Assemble.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index ac1115d..4ded58c 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1375,6 +1375,7 @@ int Assemble(struct supertype *st, char *mddev,
 							sysfs_set_num(sra, NULL,
 								      "stripe_cache_size",
 								      (4 * content->array.chunk_size / 4096) + 1);
+						sysfs_free(sra);
 					}
 				}
 				if (okcnt < (unsigned)content->array.raid_disks) {
-- 
1.7.6.4

[PATCH 03/11] Fix memory leak

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Detail.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Detail.c b/Detail.c
index ca34abe..c564786 100644
--- a/Detail.c
+++ b/Detail.c
@@ -587,6 +587,7 @@ This is pretty boring
 out:
 	close(fd);
 	free(subarray);
+	sysfs_free(sra);
 	return rv;
 }
 
-- 
1.7.6.4

[PATCH 04/11] Fix memory leak of 'st3' in array_try_spare()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Incremental.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index 98a3a74..7e345c5 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -956,8 +956,10 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 			 * to obtain minimum spare size */
 			struct supertype *st3 = dup_super(st2);
 			int mdfd = open_dev(mp->devnum);
-			if (!mdfd)
+			if (!mdfd) {
+				free(st3);
 				goto next;
+			}
 			if (st3->ss->load_container &&
 			    !st3->ss->load_container(st3, mdfd, mp->path)) {
 				component_size = st3->ss->min_acceptable_spare_size(st3);
-- 
1.7.6.4

[PATCH 05/11] partition_try_spare() use closedir() to release DIR * returned by opendir()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Incremental.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index 7e345c5..cc50a79 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -1199,6 +1199,8 @@ static int partition_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 			close(fd);
 	}
 
+	closedir(dir);
+
 	if (!chosen)
 		return 1;
 
-- 
1.7.6.4

[PATCH 06/11] Fix memory leak

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super-gpt.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/super-gpt.c b/super-gpt.c
index 6f852aa..b8c9aae 100644
--- a/super-gpt.c
+++ b/super-gpt.c
@@ -179,8 +179,10 @@ static struct supertype *match_metadata_desc(char *arg)
 
 	if (!st)
 		return st;
-	if (strcmp(arg, "gpt") != 0)
+	if (strcmp(arg, "gpt") != 0) {
+		free(st);
 		return NULL;
+	}
 
 	st->ss = &gpt;
 	st->info = NULL;
-- 
1.7.6.4

[PATCH 07/11] Add missing return in case of trying to grow sub-array

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Grow.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Grow.c b/Grow.c
index 05350dd..4ac24a1 100644
--- a/Grow.c
+++ b/Grow.c
@@ -134,6 +134,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
 		fprintf(stderr, Name ": Cannot grow linear sub-arrays yet\n");
 		free(subarray);
 		free(st);
+		return 1;
 	}
 
 	nfd = open(newdev, O_RDWR|O_EXCL|O_DIRECT);
-- 
1.7.6.4

[PATCH 08/11] Avoid memory leak

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

In case of second posix_memalign() failing, release memory allocated
in first posix_memalign() call.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super-intel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 3525dae..1caee70 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4200,6 +4200,7 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
 				__func__);
 			free(super->buf);
 			free(super);
+			free(mpb_new);
 			return 0;
 		}
 		memcpy(mpb_new, mpb, size_old);
-- 
1.7.6.4

[PATCH 09/11] policy_add(): Add missing va_end()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 policy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/policy.c b/policy.c
index 7959c97..1114286 100644
--- a/policy.c
+++ b/policy.c
@@ -510,6 +510,7 @@ void policy_add(char *type, ...)
 	}
 	pr->next = config_rules;
 	config_rules = pr;
+	va_end(ap);
 }
 
 void policy_free(void)
-- 
1.7.6.4

[PATCH 10/11] Write_rules(): Avoid stack corruption if using extremely long udev pathname

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 policy.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/policy.c b/policy.c
index 1114286..cd260c6 100644
--- a/policy.c
+++ b/policy.c
@@ -883,7 +883,8 @@ int Write_rules(char *rule_name)
        char udev_rule_file[PATH_MAX];
 
        if (rule_name) {
-	       strcpy(udev_rule_file, rule_name);
+	       strncpy(udev_rule_file, rule_name, sizeof(udev_rule_file) - 6);
+	       udev_rule_file[sizeof(udev_rule_file) - 6] = '\0';
 	       strcat(udev_rule_file, ".temp");
                fd = creat(udev_rule_file,
                           S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-- 
1.7.6.4

[PATCH 11/11] mdmon(): Error out if failing to connect to victim monitor

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdmon.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index ee68e3c..bdcda0e 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -457,8 +457,14 @@ static int mdmon(char *devname, int devnum, int must_fork, int takeover)
 	sigaction(SIGPIPE, &act, NULL);
 
 	victim = mdmon_pid(container->devnum);
-	if (victim >= 0)
+	if (victim >= 0) {
 		victim_sock = connect_monitor(container->devname);
+		if (victim_sock < 0) {
+			fprintf(stderr, "mdmon: %s unable to connect monitor\n",
+				container->devname);
+			exit(3);
+		}
+	}
 
 	ignore = chdir("/");
 	if (!takeover && victim > 0 && victim_sock >= 0) {
-- 
1.7.6.4

Re: [PATCH 00/11] Memory/resource leaks and unchecked return fixes

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

On Mon, 31 Oct 2011 15:02:28 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> This is another pile of patches to fixup memory leaks and buffer
> overflows found in the coverity run. 

Thanks Jes,
  I have applied all of these.

NeilBrown



> 
> Cheers,
> Jes
> 
> 
> Jes Sorensen (11):
>   Fix memory leaks in reshape_array()
>   Fix memory leak
>   Fix memory leak
>   Fix memory leak of 'st3' in array_try_spare()
>   partition_try_spare() use closedir() to release DIR * returned by
>     opendir()
>   Fix memory leak
>   Add missing return in case of trying to grow sub-array
>   Avoid memory leak
>   policy_add(): Add missing va_end()
>   Write_rules(): Avoid stack corruption if using extremely long udev
>     pathname
>   mdmon(): Error out if failing to connect to victim monitor
> 
>  Assemble.c    |    1 +
>  Detail.c      |    1 +
>  Grow.c        |   16 ++++++++++++++--
>  Incremental.c |    6 +++++-
>  mdmon.c       |    8 +++++++-
>  policy.c      |    4 +++-
>  super-gpt.c   |    4 +++-
>  super-intel.c |    1 +
>  8 files changed, 35 insertions(+), 6 deletions(-)
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]