gfs2-utils: master - fsck.gfs2: Add return code checks and initializations

Bob Peterson rpeterso at fedoraproject.org
Tue Sep 6 14:04:01 UTC 2011


Gitweb:        http://git.fedorahosted.org/git/gfs2-utils.git?p=gfs2-utils.git;a=commitdiff;h=355e6e070ed0763312bf3ac149ac8ab021e9d7c3
Commit:        355e6e070ed0763312bf3ac149ac8ab021e9d7c3
Parent:        3406d7e9c36f2ecab9dbceefbacb9f8d0ff430db
Author:        Bob Peterson <rpeterso at redhat.com>
AuthorDate:    Tue Sep 6 08:07:55 2011 -0500
Committer:     Bob Peterson <rpeterso at redhat.com>
CommitterDate: Tue Sep 6 08:07:55 2011 -0500

fsck.gfs2: Add return code checks and initializations

This patch fixes several problems detected by the coverity
tool whereby variables could be accessed without being
initialized, and where return codes should be checked and
were not.

rhbz#675723
---
 gfs2/fsck/fs_recovery.c |    9 +++++++++
 gfs2/fsck/initialize.c  |   29 +++++++++++++++++++++--------
 gfs2/fsck/metawalk.c    |    9 ++++++---
 gfs2/fsck/pass1.c       |   19 +++++++++++++------
 gfs2/fsck/pass1b.c      |    2 +-
 gfs2/fsck/pass2.c       |    2 +-
 gfs2/fsck/pass5.c       |   24 +++++++++++++++++++-----
 7 files changed, 70 insertions(+), 24 deletions(-)

diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 7bee86a..8c3062b 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -127,6 +127,10 @@ static int buf_lo_scan_elements(struct gfs2_inode *ip, unsigned int start,
 			  (unsigned long long)blkno, (unsigned long long)blkno,
 			  start);
 		bh_ip = bget(sdp, blkno);
+		if (!bh_ip) {
+			log_err(_("Out of memory when replaying journals.\n"));
+			return FSCK_ERROR;
+		}
 		memcpy(bh_ip->b_data, bh_log->b_data, sdp->bsize);
 
 		check_magic = ((struct gfs2_meta_header *)
@@ -235,6 +239,10 @@ static int databuf_lo_scan_elements(struct gfs2_inode *ip, unsigned int start,
 			  (unsigned long long)blkno, (unsigned long long)blkno,
 			  start);
 		bh_ip = bget(sdp, blkno);
+		if (!bh_ip) {
+			log_err(_("Out of memory when replaying journals.\n"));
+			return FSCK_ERROR;
+		}
 		memcpy(bh_ip->b_data, bh_log->b_data, sdp->bsize);
 
 		/* Unescape */
@@ -359,6 +367,7 @@ static int fix_journal_seq_no(struct gfs2_inode *ip)
 	uint32_t extlen;
 	struct gfs2_buffer_head *bh;
 
+	memset(&lh, 0, sizeof(lh));
 	for (blk = 0; blk < jd_blocks; blk++) {
 		error = get_log_header(ip, blk, &lh);
 		if (error == 1) /* if not a log header */
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 3483560..ba0df41 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -597,7 +597,7 @@ static int fetch_rgrps(struct gfs2_sbd *sdp)
  */
 static int init_system_inodes(struct gfs2_sbd *sdp)
 {
-	uint64_t inumbuf;
+	uint64_t inumbuf = 0;
 	char *buf;
 	struct gfs2_statfs_change sc;
 	uint64_t addl_mem_needed;
@@ -644,8 +644,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 			}
 		}
 		/* Read inum entry into buffer */
-		gfs2_readi(sdp->md.inum, &inumbuf, 0,
-			   sdp->md.inum->i_di.di_size);
+		err = gfs2_readi(sdp->md.inum, &inumbuf, 0,
+				 sdp->md.inum->i_di.di_size);
+		if (err != sdp->md.inum->i_di.di_size) {
+			log_crit(_("Error %d reading system inum inode. "
+				   "Aborting.\n"), err);
+			goto fail;
+		}
 		/* call gfs2_inum_range_in() to retrieve range */
 		sdp->md.next_inum = be64_to_cpu(inumbuf);
 	}
@@ -678,8 +683,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	if (sdp->md.statfs->i_di.di_size) {
 		buf = malloc(sdp->md.statfs->i_di.di_size);
 		if (buf) {
-			gfs2_readi(sdp->md.statfs, buf, 0,
-				   sdp->md.statfs->i_di.di_size);
+			err = gfs2_readi(sdp->md.statfs, buf, 0,
+					 sdp->md.statfs->i_di.di_size);
+			if (err != sdp->md.statfs->i_di.di_size) {
+				log_crit(_("Error %d reading statfs file. "
+					   "Aborting.\n"), err);
+				goto fail;
+			}
 			/* call gfs2_inum_range_in() to retrieve range */
 			gfs2_statfs_change_in(&sc, buf);
 			free(buf);
@@ -1305,14 +1315,17 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum,
  */
 static int reconstruct_journals(struct gfs2_sbd *sdp)
 {
-	int i;
+	int i, count;
 	struct gfs_jindex ji;
 	char buf[sizeof(struct gfs_jindex)];
 
 	log_err("Clearing GFS journals (this may take a while)\n");
 	for (i = 0; i < sdp->md.journals; i++) {
-		gfs2_readi(sdp->md.jiinode, buf, i * sizeof(struct gfs_jindex),
-			   sizeof(struct gfs_jindex));
+		count = gfs2_readi(sdp->md.jiinode, buf,
+				   i * sizeof(struct gfs_jindex),
+				   sizeof(struct gfs_jindex));
+		if (count != sizeof(struct gfs_jindex))
+			return 0;
 		gfs_jindex_in(&ji, buf);
 		if ((i % 2) == 0)
 			log_err(".");
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 7adf73d..ef7f05d 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1534,7 +1534,8 @@ static int alloc_metalist(struct gfs2_inode *ip, uint64_t block,
 	   after the bitmap has been set but before the blockmap has. */
 	*bh = bread(ip->i_sbd, block);
 	q = block_type(block);
-	if (blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE) {
+	if (q < 0 ||
+	    blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE) {
 		log_debug(_("%s reference to new metadata block "
 			    "%lld (0x%llx) is now marked as indirect.\n"),
 			  desc, (unsigned long long)block,
@@ -1553,7 +1554,8 @@ static int alloc_data(struct gfs2_inode *ip, uint64_t block, void *private)
 	/* We can't check the bitmap here because this function is called
 	   after the bitmap has been set but before the blockmap has. */
 	q = block_type(block);
-	if (blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE) {
+	if (q < 0 ||
+	    blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE) {
 		log_debug(_("%s reference to new data block "
 			    "%lld (0x%llx) is now marked as data.\n"),
 			  desc, (unsigned long long)block,
@@ -1571,7 +1573,8 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
 	/* We can't check the bitmap here because this function is called
 	   after the bitmap has been set but before the blockmap has. */
 	q = block_type(block);
-	if (blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE)
+	if (q < 0 ||
+	    blockmap_to_bitmap(q, ip->i_sbd->gfs1) == GFS2_BLKST_FREE)
 		fsck_blockmap_set(ip, block, _("newly allocated leaf"),
 				  gfs2_leaf_blk);
 	return 0;
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 60b82d0..18757fe 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -1328,7 +1328,7 @@ static int check_system_inode(struct gfs2_sbd *sdp,
 {
 	uint64_t iblock = 0;
 	struct dir_status ds = {0};
-	int error;
+	int error, err = 0;
 
 	log_info( _("Checking system inode '%s'\n"), filename);
 	if (*sysinode) {
@@ -1403,12 +1403,19 @@ static int check_system_inode(struct gfs2_sbd *sdp,
 		sysdir_fxns.private = &bc;
 		if ((*sysinode)->i_di.di_flags & GFS2_DIF_EXHASH)
 			check_metatree(*sysinode, &sysdir_fxns);
-		else
-			check_linear_dir(*sysinode, (*sysinode)->i_bh,
-					 &sysdir_fxns);
+		else {
+			err = check_linear_dir(*sysinode, (*sysinode)->i_bh,
+					       &sysdir_fxns);
+			/* If we encountered an error in our directory check
+			   we should still call handle_ip, but return the
+			   error later. */
+			if (err)
+				log_err(_("Error found in %s while checking "
+					  "directory entries.\n"), filename);
+		}
 	}
 	error = handle_ip(sdp, *sysinode);
-	return error;
+	return error ? error : err;
 }
 
 static int build_a_journal(struct gfs2_sbd *sdp)
@@ -1540,7 +1547,7 @@ int pass1(struct gfs2_sbd *sdp)
 {
 	struct osi_node *n, *next = NULL;
 	struct gfs2_buffer_head *bh;
-	uint64_t block;
+	uint64_t block = 0;
 	struct rgrp_tree *rgd;
 	int first;
 	uint64_t i;
diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c
index ae6d45c..1879cc4 100644
--- a/gfs2/fsck/pass1b.c
+++ b/gfs2/fsck/pass1b.c
@@ -701,7 +701,7 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *b)
 		ip = fsck_load_inode(sdp, id->block_no);
 
 		q = block_type(id->block_no);
-		if (q == gfs2_inode_invalid) {
+		if (q < 0 || q == gfs2_inode_invalid) {
 			log_debug( _("The remaining reference inode %lld "
 				     "(0x%llx) is marked invalid: Marking "
 				     "the block as free.\n"),
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index d89dd4f..8562b6a 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -328,7 +328,7 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
 	 * 2. Blocks marked "bad" need to have their entire
 	 * metadata tree deleted.
 	*/
-	if (q == gfs2_inode_invalid || q == gfs2_bad_block) {
+	if (q < 0 || q == gfs2_inode_invalid || q == gfs2_bad_block) {
 		/* This entry's inode has bad blocks in it */
 
 		/* Handle bad blocks */
diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c
index 340df20..cb18b5a 100644
--- a/gfs2/fsck/pass5.c
+++ b/gfs2/fsck/pass5.c
@@ -99,14 +99,14 @@ static int gfs2_convert_mark(uint8_t q, uint32_t *count)
 	return -1;
 }
 
-
 static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
 			      unsigned int buflen, uint64_t *rg_block,
 			      uint64_t rg_data, uint32_t *count)
 {
 	unsigned char *byte, *end;
 	unsigned int bit;
-	unsigned char rg_status, block_status;
+	unsigned char rg_status;
+	int block_status;
 	uint8_t q;
 	uint64_t block;
 
@@ -121,13 +121,26 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
 		warm_fuzzy_stuff(block);
 		if (skip_this_pass || fsck_abort) /* if asked to skip the rest */
 			return 0;
+
 		q = block_type(block);
+		if (q < 0) {
+			log_err( _("Invalid block type for block #%llu "
+				   "(0x%llx)\n"), (unsigned long long)block,
+				 (unsigned long long)block);
+			return q;
+		}
 
 		if (sdp->gfs1)
 			block_status = gfs1_convert_mark(q, count);
 		else
 			block_status = gfs2_convert_mark(q, count);
 
+		if (block_status < 0) {
+			log_err( _("Invalid status for block %llu (0x%llx).\n"),
+				 (unsigned long long)block,
+				 (unsigned long long)block);
+			return block_status;
+		}
 		/* If one node opens a file and another node deletes it, we
 		   may be left with a block that appears to be "unlinked" in
 		   the bitmap, but nothing links to it. This is a valid case
@@ -208,9 +221,10 @@ static void update_rgrp(struct gfs2_sbd *sdp, struct rgrp_tree *rgp,
 		bits = &rgp->bits[i];
 
 		/* update the bitmaps */
-		check_block_status(sdp, rgp->bh[i]->b_data + bits->bi_offset,
-				   bits->bi_len, &rg_block, rgp->ri.ri_data0,
-				   count);
+		if (check_block_status(sdp, rgp->bh[i]->b_data +
+				       bits->bi_offset, bits->bi_len,
+				       &rg_block, rgp->ri.ri_data0, count))
+			return;
 		if (skip_this_pass || fsck_abort) /* if asked to skip the rest */
 			return;
 	}


More information about the cluster-commits mailing list