[Cluster-devel] [PATCH 1/3] libgfscontrol: Fix resource leaks

[Cluster-devel] [PATCH 1/3] libgfscontrol: Fix resource leaks

[Andrew Price]

Spotted by coverity:
- Variable "reply" going out of scope leaks the storage it points to (3
  occurrences) and
- Handle variable "fd" going out of scope leaks the handle.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 group/libgfscontrol/main.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/group/libgfscontrol/main.c b/group/libgfscontrol/main.c
index c6eaf97..96a8e03 100644
--- a/group/libgfscontrol/main.c
+++ b/group/libgfscontrol/main.c
@@ -108,7 +108,7 @@ static int do_dump(int cmd, char *name, char *buf)
 	fd = do_connect(GFSC_QUERY_SOCK_PATH);
 	if (fd < 0) {
 		rv = fd;
-		goto out;
+		goto out_free;
 	}
 
 	rv = do_write(fd, &h, sizeof(h));
@@ -127,6 +127,8 @@ static int do_dump(int cmd, char *name, char *buf)
 	       GFSC_DUMP_SIZE);
  out_close:
 	close(fd);
+ out_free:
+	free(reply);
  out:
 	return rv;
 }
@@ -238,7 +240,7 @@ int gfsc_mountgroups(int max, int *count, struct gfsc_mountgroup *mgs)
 	fd = do_connect(GFSC_QUERY_SOCK_PATH);
 	if (fd < 0) {
 		rv = fd;
-		goto out;
+		goto out_free;
 	}
 
 	rv = do_write(fd, &h, sizeof(h));
@@ -268,6 +270,8 @@ int gfsc_mountgroups(int max, int *count, struct gfsc_mountgroup *mgs)
 	       mg_count * sizeof(struct gfsc_mountgroup));
  out_close:
 	close(fd);
+ out_free:
+	free(reply);
  out:
 	return rv;
 }
@@ -296,7 +300,7 @@ int gfsc_mountgroup_nodes(char *name, int type, int max, int *count,
 	fd = do_connect(GFSC_QUERY_SOCK_PATH);
 	if (fd < 0) {
 		rv = fd;
-		goto out;
+		goto out_free;
 	}
 
 	rv = do_write(fd, &h, sizeof(h));
@@ -326,6 +330,8 @@ int gfsc_mountgroup_nodes(char *name, int type, int max, int *count,
 	       node_count * sizeof(struct gfsc_node));
  out_close:
 	close(fd);
+ out_free:
+	free(reply);
  out:
 	return rv;
 }
@@ -409,7 +415,7 @@ int gfsc_fs_leave(struct gfsc_mount_args *ma, int reason)
 	char msg[sizeof(struct gfsc_header) + sizeof(struct gfsc_mount_args)];
 	struct gfsc_header *h = (struct gfsc_header *)msg;
 	char *name = strstr(ma->table, ":") + 1;
-	int fd;
+	int fd, err;
 
 	init_header(h, GFSC_CMD_FS_LEAVE, name,
 		    sizeof(struct gfsc_mount_args));
@@ -423,6 +429,8 @@ int gfsc_fs_leave(struct gfsc_mount_args *ma, int reason)
 	if (fd < 0)
 		return fd;
 
-	return do_write(fd, msg, sizeof(msg));
+	err = do_write(fd, msg, sizeof(msg));
+	close(fd);
+	return err;
 }
 
-- 
1.7.7.5

[Cluster-devel] [PATCH 2/3] fsck.gfs2: Plug a leak in peruse_system_dinode()

[Andrew Price]

Spotted by coverity: Variable "ip" going out of scope leaks the storage
it points to.

If one of the if/else conditions is true, ip is stored in a global
variable or freed, so the leak happens when none of them are true. To
fix this I've added an else statement to free ip and moved the
out_discard_ip label into it to avoid duplicating the inode_put().

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/initialize.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 3daf12d..f07e0b2 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -911,11 +911,10 @@ static void peruse_system_dinode(struct gfs2_sbd *sdp, struct gfs2_dinode *di,
 		fix_md.qinode = ip;
 		log_warn(_("Found system quota file at: 0x%llx\n"),
 			 di->di_num.no_addr);
-	}
-	return;
-
+	} else {
 out_discard_ip:
-	inode_put(&ip);
+		inode_put(&ip);
+	}
 }
 
 /**
-- 
1.7.7.5

[Cluster-devel] [PATCH 3/3] fsck.gfs2: Fix unchecked malloc in gfs2_dup_set()

[Andrew Price]

Spotted by coverity: Dereferencing a pointer that might be null "data"
when calling "memset"

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/util.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
index f37fe7d..6c80ae8 100644
--- a/gfs2/fsck/util.c
+++ b/gfs2/fsck/util.c
@@ -253,6 +253,10 @@ static struct duptree *gfs2_dup_set(uint64_t dblock, int create)
 	if (!create)
 		return NULL;
 	data = malloc(sizeof(struct duptree));
+	if (data == NULL) {
+		log_crit( _("Unable to allocate duptree structure\n"));
+		return NULL;
+	}
 	dups_found++;
 	memset(data, 0, sizeof(struct duptree));
 	/* Add new node and rebalance tree. */
-- 
1.7.7.5

[Cluster-devel] [PATCH 3/3] fsck.gfs2: Fix unchecked malloc in gfs2_dup_set()

[Steven Whitehouse]

Hi,

Those look good to me,

Steve.

On Fri, 2012-01-20 at 12:39 +0000, Andrew Price wrote:
> Spotted by coverity: Dereferencing a pointer that might be null "data"
> when calling "memset"
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/fsck/util.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
> index f37fe7d..6c80ae8 100644
> --- a/gfs2/fsck/util.c
> +++ b/gfs2/fsck/util.c
> @@ -253,6 +253,10 @@ static struct duptree *gfs2_dup_set(uint64_t dblock, int create)
>  	if (!create)
>  		return NULL;
>  	data = malloc(sizeof(struct duptree));
> +	if (data == NULL) {
> +		log_crit( _("Unable to allocate duptree structure\n"));
> +		return NULL;
> +	}
>  	dups_found++;
>  	memset(data, 0, sizeof(struct duptree));
>  	/* Add new node and rebalance tree. */