[Cluster-devel] [PATCH 01/11] gfs2_edit: Remove a useless assignment

[Cluster-devel] [PATCH 01/11] gfs2_edit: Remove a useless assignment

[Andrew Price]

Spotted by coverity: Pointer "n" returned by "osi_prev(n)" is never used

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

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index ed9a7b0..9f7b2eb 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -710,7 +710,6 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 			slow = ri_update(&sbd, 0, &rgcount, &sane);
 		n = osi_last(&sbd.rgtree);
 		last_rgd = (struct rgrp_tree *)n;
-		n = osi_prev(n);
 		fssize = last_rgd->ri.ri_addr + rgrp_size(last_rgd);
 		last_fs_block = fssize;
 		fssize *= sbd.bsize;
-- 
1.7.6.4

[Cluster-devel] [PATCH 02/11] gfs2_edit: Check return value of compute_constants

[Andrew Price]

Spotted by coverity: Calling function "compute_constants" without
checking return value.

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 73fb39f..fcab625 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1276,7 +1276,10 @@ static void read_superblock(int fd)
 		perror(sbd.device_name);
 		exit(-1);
 	}
-	compute_constants(&sbd);
+	if(compute_constants(&sbd)) {
+		fprintf(stderr, "Failed to compute constants.\n");
+		exit(-1);
+	}
 	if (sbd.gfs1 || (sbd.sd_sb.sb_header.mh_magic == GFS2_MAGIC &&
 		     sbd.sd_sb.sb_header.mh_type == GFS2_METATYPE_SB))
 		block = 0x10 * (GFS2_DEFAULT_BSIZE / sbd.bsize);
-- 
1.7.6.4

[Cluster-devel] [PATCH 03/11] gfs2_edit: Fix possible uninitialized access

[Andrew Price]

Spotted by coverity: Using uninitialized value "ch".

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/hexedit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index fcab625..72ee7d2 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -2232,7 +2232,7 @@ static void process_field(const char *field, const char *nstr)
 /* ------------------------------------------------------------------------ */
 static void interactive_mode(void)
 {
-	int ch, Quit;
+	int ch = 0, Quit;
 
 	if ((wind = initscr()) == NULL) {
 		fprintf(stderr, "Error: unable to initialize screen.");
-- 
1.7.6.4

[Cluster-devel] [PATCH 04/11] gfs2_edit: Fix memory leak in dump_journal()

[Andrew Price]

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

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 72ee7d2..b05a7d6 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -2775,6 +2775,7 @@ static void dump_journal(const char *journal)
 			block = saveblk;
 		}
 	}
+	inode_put(&j_inode);
 	brelse(j_bh);
 	blockhist = -1; /* So we don't print anything else */
 	free(jbuf);
-- 
1.7.6.4

[Cluster-devel] [PATCH 05/11] gfs2_edit: Fix null pointer dereference in dump_journal

[Andrew Price]

Spotted by coverity: Dereferencing a pointer that might be null "(void
*)jbuf" when calling "fsck_readi"

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index b05a7d6..7a17c04 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -2672,6 +2672,10 @@ static void dump_journal(const char *journal)
 		j_bh = bread(&sbd, jblock);
 		j_inode = inode_get(&sbd, j_bh);
 		jbuf = malloc(sbd.bsize);
+		if (jbuf == NULL) {
+			fprintf(stderr, "Out of memory\n");
+			exit(-1);
+		}
 	}
 
 	for (jb = 0; jb < j_size; jb += (sbd.gfs1 ? 1:sbd.bsize)) {
-- 
1.7.6.4

[Cluster-devel] [PATCH 06/11] gfs2_edit: Remove unused j_inode from find_journal_block()

[Andrew Price]

Spotted by coverity: Pointer "j_inode" returned by "inode_get(&sbd,
j_bh)" is never used

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 7a17c04..1deb374 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1493,7 +1493,6 @@ static uint64_t find_journal_block(const char *journal, uint64_t *j_size)
 	int amtread;
 	struct gfs2_buffer_head *jindex_bh, *j_bh;
 	char jbuf[sbd.bsize];
-	struct gfs2_inode *j_inode = NULL;
 
 	journal_num = atoi(journal + 7);
 	/* Figure out the block of the jindex file */
@@ -1527,7 +1526,6 @@ static uint64_t find_journal_block(const char *journal, uint64_t *j_size)
 
 		jblock = indirect->ii[0].dirent[journal_num + 2].block;
 		j_bh = bread(&sbd, jblock);
-		j_inode = inode_get(&sbd, j_bh);
 		gfs2_dinode_in(&jdi, j_bh);/* parse dinode to struct */
 		*j_size = jdi.di_size;
 		brelse(j_bh);
-- 
1.7.6.4

[Cluster-devel] [PATCH 07/11] gfs2_edit: Fix memory leak in find_journal_block

[Andrew Price]

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

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 1deb374..f58657b 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1521,6 +1521,7 @@ static uint64_t find_journal_block(const char *journal, uint64_t *j_size)
 			jblock = ji.ji_addr;
 			*j_size = ji.ji_nsegment * 0x10;
 		}
+		inode_put(&jiinode);
 	} else {
 		struct gfs2_dinode jdi;
 
-- 
1.7.6.4

[Cluster-devel] [PATCH 08/11] gfs2_edit: Check for error value from gfs2_get_bitmap

[Andrew Price]

Spotted by coverity: gfs2_get_bitmap returns -1 on error and it was
being used as an array index in display_block_type.

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index f58657b..02a4091 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -756,6 +756,10 @@ int display_block_type(int from_restore)
 				print_gfs2(" blk ");
 				for (b = blknum; b < blknum + 4; b++) {
 					btype = gfs2_get_bitmap(&sbd, b, rgd);
+					if (btype < 0) {
+						fprintf(stderr, "Failed to retrieve block state from bitmap\n");
+						exit(-1);
+					}
 					print_gfs2("0x%x-%s  ", b,
 						   allocdesc[sbd.gfs1][btype]);
 				}
-- 
1.7.6.4

[Cluster-devel] [PATCH 09/11] gfs2_edit: Fix resource leaks in display_extended()

[Andrew Price]

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

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

diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
index 7fd770f..7c0ced5 100644
--- a/gfs2/edit/extended.c
+++ b/gfs2/edit/extended.c
@@ -669,30 +669,35 @@ int display_extended(void)
 			tmp_bh = bread(&sbd, masterblock("rindex"));
 		tmp_inode = inode_get(&sbd, tmp_bh);
 		parse_rindex(tmp_inode, FALSE);
+		inode_put(&tmp_inode);
 		brelse(tmp_bh);
 	}
 	else if (block_is_jindex()) {
 		tmp_bh = bread(&sbd, block);
 		tmp_inode = inode_get(&sbd, tmp_bh);
 		print_jindex(tmp_inode);
+		inode_put(&tmp_inode);
 		brelse(tmp_bh);
 	}
 	else if (block_is_inum_file()) {
 		tmp_bh = bread(&sbd, block);
 		tmp_inode = inode_get(&sbd, tmp_bh);
 		print_inum(tmp_inode);
+		inode_put(&tmp_inode);
 		brelse(tmp_bh);
 	}
 	else if (block_is_statfs_file()) {
 		tmp_bh = bread(&sbd, block);
 		tmp_inode = inode_get(&sbd, tmp_bh);
 		print_statfs(tmp_inode);
+		inode_put(&tmp_inode);
 		brelse(tmp_bh);
 	}
 	else if (block_is_quota_file()) {
 		tmp_bh = bread(&sbd, block);
 		tmp_inode = inode_get(&sbd, tmp_bh);
 		print_quota(tmp_inode);
+		inode_put(&tmp_inode);
 		brelse(tmp_bh);
 	}
 	return 0;
-- 
1.7.6.4

[Cluster-devel] [PATCH 10/11] gfs2_edit: Fix resource leak in print_block_details()

[Andrew Price]

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

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

diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
index 7c0ced5..1066e1b 100644
--- a/gfs2/edit/extended.c
+++ b/gfs2/edit/extended.c
@@ -396,6 +396,7 @@ static void print_block_details(struct iinfo *ind, int level, int cur_height,
 	if (!tmpbuf) {
 		fprintf(stderr, "Out of memory in function "
 			"display_indirect\n");
+		free(more_indir);
 		return;
 	}
 	while (thisblk) {
-- 
1.7.6.4

[Cluster-devel] [PATCH 11/11] gfs2_edit: Fix null pointer derefs in display_block_type()

[Andrew Price]

Spotted by coverity: Dereferencing null variable "rgd".

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

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 02a4091..69d499e 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -713,7 +713,7 @@ int display_block_type(int from_restore)
 		if ((be32_to_cpu(mh->mh_type) == GFS2_METATYPE_RG)) {
 			int ptroffset = edit_row[dmode] * 16 + edit_col[dmode];
 
-			if (ptroffset >= struct_len || pgnum) {
+			if (rgd && (ptroffset >= struct_len || pgnum)) {
 				int blknum, b, btype;
 
 				blknum = pgnum * screen_chunk_size;
@@ -731,7 +731,7 @@ int display_block_type(int from_restore)
 		} else if ((be32_to_cpu(mh->mh_type) == GFS2_METATYPE_RB)) {
 			int ptroffset = edit_row[dmode] * 16 + edit_col[dmode];
 
-			if (ptroffset >= struct_len || pgnum) {
+			if (rgd && (ptroffset >= struct_len || pgnum)) {
 				int blknum, b, btype, rb_number;
 
 				rb_number = block - rgd->ri.ri_addr;
-- 
1.7.6.4

[Cluster-devel] [PATCH 11/11] gfs2_edit: Fix null pointer derefs in display_block_type()

[Steven Whitehouse]

Hi,

Those all look good to me. ACK,

Steve.

On Fri, 2012-01-06 at 15:19 +0000, Andrew Price wrote:
> Spotted by coverity: Dereferencing null variable "rgd".
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/edit/hexedit.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index 02a4091..69d499e 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -713,7 +713,7 @@ int display_block_type(int from_restore)
>  		if ((be32_to_cpu(mh->mh_type) == GFS2_METATYPE_RG)) {
>  			int ptroffset = edit_row[dmode] * 16 + edit_col[dmode];
>  
> -			if (ptroffset >= struct_len || pgnum) {
> +			if (rgd && (ptroffset >= struct_len || pgnum)) {
>  				int blknum, b, btype;
>  
>  				blknum = pgnum * screen_chunk_size;
> @@ -731,7 +731,7 @@ int display_block_type(int from_restore)
>  		} else if ((be32_to_cpu(mh->mh_type) == GFS2_METATYPE_RB)) {
>  			int ptroffset = edit_row[dmode] * 16 + edit_col[dmode];
>  
> -			if (ptroffset >= struct_len || pgnum) {
> +			if (rgd && (ptroffset >= struct_len || pgnum)) {
>  				int blknum, b, btype, rb_number;
>  
>  				rb_number = block - rgd->ri.ri_addr;