gfs2-utils: master - fsck.gfs2: Check for blocks wrongly inside resource groups

Bob Peterson rpeterso at fedoraproject.org
Tue Aug 30 20:25:15 UTC 2011


Gitweb:        http://git.fedorahosted.org/git/gfs2-utils.git?p=gfs2-utils.git;a=commitdiff;h=666afe39c6e318dfe596351232fd37586b0d5ae2
Commit:        666afe39c6e318dfe596351232fd37586b0d5ae2
Parent:        3e12755de1021c62eab9c47bf3958b29a206a653
Author:        Bob Peterson <rpeterso at redhat.com>
AuthorDate:    Mon Aug 8 12:46:29 2011 -0500
Committer:     Bob Peterson <rpeterso at redhat.com>
CommitterDate: Mon Aug 29 12:51:38 2011 -0500

fsck.gfs2: Check for blocks wrongly inside resource groups

It's not enough to range_check blocks in order to call them valid.
We also need to check whether those block collide with resource groups.
We don't want a bitmap block to ever be referenced unless it's part of
the rgrp and rindex functions.  This patch changes most of the fsck code
from doing simple block range checks to doing range checks plus checks
for blocks inside the resource groups.

rhbz#675723
---
 gfs2/fsck/lost_n_found.c |    2 +-
 gfs2/fsck/metawalk.c     |   20 +++++++-------
 gfs2/fsck/pass1.c        |   66 +++++++++++++++++++++++----------------------
 gfs2/fsck/pass1b.c       |    2 +-
 gfs2/fsck/pass1c.c       |   10 +++---
 gfs2/fsck/pass2.c        |    4 +-
 gfs2/fsck/rgrepair.c     |    2 +-
 gfs2/fsck/util.c         |    2 +-
 gfs2/libgfs2/fs_bits.c   |   19 ++++++++++++-
 gfs2/libgfs2/libgfs2.h   |    1 +
 10 files changed, 74 insertions(+), 54 deletions(-)

diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index 04aa90d..4eff83b 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -104,7 +104,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){
 		/* If there's a pre-existing .. directory entry, we have to
 		   back out the links. */
 		di = dirtree_find(ip->i_di.di_num.no_addr);
-		if (di && gfs2_check_range(sdp, di->dotdot_parent) == 0) {
+		if (di && !valid_block(sdp, di->dotdot_parent) == 0) {
 			struct gfs2_inode *dip;
 
 			log_debug(_("Directory %lld (0x%llx) already had a "
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 3cee0fd..5d0afa5 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -455,7 +455,7 @@ static void warn_and_patch(struct gfs2_inode *ip, uint64_t *leaf_no,
 	}
 	if (*leaf_no == *bad_leaf ||
 	    query( _("Attempt to patch around it? (y/n) "))) {
-		if (gfs2_check_range(ip->i_sbd, old_leaf) == 0)
+		if (!valid_block(ip->i_sbd, old_leaf) == 0)
 			gfs2_put_leaf_nr(ip, pindex, old_leaf);
 		else
 			gfs2_put_leaf_nr(ip, pindex, first_ok_leaf);
@@ -605,7 +605,7 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 	first_ok_leaf = leaf_no = -1;
 	for(lindex = 0; lindex < (1 << ip->i_di.di_depth); lindex++) {
 		gfs2_get_leaf_nr(ip, lindex, &leaf_no);
-		if (gfs2_check_range(ip->i_sbd, leaf_no) == 0) {
+		if (!valid_block(ip->i_sbd, leaf_no) == 0) {
 			lbh = bread(sdp, leaf_no);
 			/* Make sure it's really a valid leaf block. */
 			if (gfs2_check_meta(lbh, GFS2_METATYPE_LF) == 0) {
@@ -644,7 +644,7 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 		}
 
 		do {
-			if (gfs2_check_range(ip->i_sbd, old_leaf) == 0) {
+			if (!valid_block(ip->i_sbd, old_leaf) == 0) {
 				error = check_num_ptrs(ip, old_leaf,
 						       &ref_count, &exp_count,
 						       &lindex, &oldleaf);
@@ -656,7 +656,7 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 			if (fsck_abort)
 				break;
 			/* Make sure the block number is in range. */
-			if (gfs2_check_range(ip->i_sbd, leaf_no)){
+			if (!valid_block(ip->i_sbd, leaf_no)){
 				log_err( _("Leaf block #%llu (0x%llx) is out "
 					"of range for directory #%llu (0x%llx"
 					").\n"), (unsigned long long)leaf_no,
@@ -909,7 +909,7 @@ int delete_block(struct gfs2_inode *ip, uint64_t block,
 		 struct gfs2_buffer_head **bh, const char *btype,
 		 void *private)
 {
-	if (gfs2_check_range(ip->i_sbd, block) == 0) {
+	if (!valid_block(ip->i_sbd, block) == 0) {
 		fsck_blockmap_set(ip, block, btype, gfs2_block_free);
 		return 0;
 	}
@@ -930,7 +930,7 @@ static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block,
 	uint8_t q;
 	struct duptree *d;
 
-	if (gfs2_check_range(ip->i_sbd, block) != 0)
+	if (!valid_block(ip->i_sbd, block) != 0)
 		return -EFAULT;
 
 	q = block_type(block);
@@ -1190,7 +1190,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
 						   (unsigned long long)block);
 					continue;
 				}
-				if (gfs2_check_range(ip->i_sbd, block)) {
+				if (!valid_block(ip->i_sbd, block)) {
 					log_debug( _("Skipping invalid block "
 						     "%lld (0x%llx)\n"),
 						   (unsigned long long)block,
@@ -1237,7 +1237,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
 		if (skip_this_pass || fsck_abort)
 			return error;
 		block =  be64_to_cpu(*ptr);
-		/* It's important that we don't call gfs2_check_range and
+		/* It's important that we don't call !valid_block and
 		   bypass calling check_data on invalid blocks because that
 		   would defeat the rangecheck_block related functions in
 		   pass1. Therefore the individual check_data functions
@@ -1437,8 +1437,8 @@ int remove_dentry_from_dir(struct gfs2_sbd *sdp, uint64_t dir,
 		     " (0x%llx)\n"), (unsigned long long)dentryblock,
 		  (unsigned long long)dentryblock,
 		  (unsigned long long)dir, (unsigned long long)dir);
-	if (gfs2_check_range(sdp, dir)) {
-		log_err( _("Parent directory out of range\n"));
+	if (!valid_block(sdp, dir)) {
+		log_err( _("Parent directory is invalid\n"));
 		return 1;
 	}
 	remove_dentry_fxns.private = &dentryblock;
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 2670d8c..e2fe73c 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -122,11 +122,11 @@ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block,
 	struct block_count *bc = (struct block_count *)private;
 
 	*bh = NULL;
-	if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */
+	if (!valid_block(ip->i_sbd, block)){ /* blk outside of FS */
 		fsck_blockmap_set(ip, ip->i_di.di_num.no_addr,
 				  _("itself"), gfs2_bad_block);
-		log_err( _("Bad indirect block pointer (out of range) "
-			   "found in system inode %lld (0x%llx).\n"),
+		log_err( _("Bad indirect block pointer (invalid or out of "
+			   "range) found in system inode %lld (0x%llx).\n"),
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr);
 		return 1;
@@ -166,10 +166,10 @@ static int resuscitate_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
 		strncpy(tmp_name, filename, de->de_name_len);
 	else
 		strncpy(tmp_name, filename, sizeof(tmp_name) - 1);
-	if (gfs2_check_range(sdp, block)) {
+	if (!valid_block(sdp, block)) {
 		log_err( _("Block # referenced by system directory entry %s "
-			   "in inode %lld (0x%llx) is out of range; "
-			   "ignored.\n"),
+			   "in inode %lld (0x%llx) is invalid or out of range;"
+			   " ignored.\n"),
 			 tmp_name, (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr);
 		return 0;
@@ -221,10 +221,10 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block,
 
 	*bh = NULL;
 
-	if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */
+	if (!valid_block(ip->i_sbd, block)){ /* blk outside of FS */
 		fsck_blockmap_set(ip, ip->i_di.di_num.no_addr,
 				  _("itself"), gfs2_bad_block);
-		log_debug( _("Bad indirect block pointer (out of range) "
+		log_debug( _("Bad indirect block (invalid/out of range) "
 			     "found in inode %lld (0x%llx).\n"),
 			   (unsigned long long)ip->i_di.di_num.no_addr,
 			   (unsigned long long)ip->i_di.di_num.no_addr);
@@ -295,7 +295,7 @@ static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block,
 
 	*bh = NULL;
 
-	if (gfs2_check_range(ip->i_sbd, block)){ /* blk outside of FS */
+	if (!valid_block(ip->i_sbd, block)){ /* blk outside of FS */
 		fsck_blockmap_set(ip, ip->i_di.di_num.no_addr,
 				  _("itself"), gfs2_block_free);
 		return 1;
@@ -350,9 +350,9 @@ static int check_data(struct gfs2_inode *ip, uint64_t block, void *private)
 	uint8_t q;
 	struct block_count *bc = (struct block_count *) private;
 
-	if (gfs2_check_range(ip->i_sbd, block)) {
+	if (!valid_block(ip->i_sbd, block)) {
 		log_err( _("inode %lld (0x%llx) has a bad data block pointer "
-			   "%lld (out of range)\n"),
+			   "%lld (invalid or out of range)\n"),
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)block);
@@ -401,12 +401,12 @@ static int undo_check_data(struct gfs2_inode *ip, uint64_t block,
 	struct duptree *d;
 	struct block_count *bc = (struct block_count *) private;
 
-	if (gfs2_check_range(ip->i_sbd, block)) {
+	if (!valid_block(ip->i_sbd, block)) {
 		/* Mark the owner of this block with the bad_block
 		 * designator so we know to check it for out of range
 		 * blocks later */
 		fsck_blockmap_set(ip, ip->i_di.di_num.no_addr,
-				  _("bad (out of range) data"),
+				  _("bad (invalid or out of range) data"),
 				  gfs2_block_free);
 		return 1;
 	}
@@ -547,7 +547,7 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t indirect,
 
 	/* This inode contains an eattr - it may be invalid, but the
 	 * eattr attributes points to a non-zero block */
-	if (gfs2_check_range(sdp, indirect)) {
+	if (!valid_block(sdp, indirect)) {
 		/* Doesn't help to mark this here - this gets checked
 		 * in pass1c */
 		return 1;
@@ -713,10 +713,10 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr,
 	struct gfs2_buffer_head *bh = NULL;
 	int error;
 
-	if (gfs2_check_range(sdp, el_blk)){
+	if (!valid_block(sdp, el_blk)){
 		log_err( _("Inode #%llu (0x%llx): Extended Attribute block "
 			   "%llu (0x%llx) has an extended leaf block #%llu "
-			   "(0x%llx) that is out of range.\n"),
+			   "(0x%llx) that is invalid or out of range.\n"),
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_eattr,
@@ -757,9 +757,10 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block,
 	}
 	if (!b || b->block != ip->i_di.di_num.no_addr)
 		gfs2_special_add(&sdp->eattr_blocks, ip->i_di.di_num.no_addr);
-	if (gfs2_check_range(sdp, block)) {
+	if (!valid_block(sdp, block)) {
 		log_warn( _("Inode #%llu (0x%llx): Extended Attribute leaf "
-			    "block #%llu (0x%llx) is out of range.\n"),
+			    "block #%llu (0x%llx) is invalid or out of "
+			    "range.\n"),
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)block, (unsigned long long)block);
@@ -825,15 +826,15 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block,
 {
 	uint8_t q;
 
-	if (gfs2_check_range(ip->i_sbd, block) != 0)
+	if (!valid_block(ip->i_sbd, block) != 0)
 		return -EFAULT;
 
 	q = block_type(block);
 	if (q != gfs2_block_free) {
 		add_duplicate_ref(ip, block, reftype, 0, INODE_INVALID);
 		log_info( _("%s block %lld (0x%llx), part of inode "
-			    "%lld (0x%llx), was free so the invalid "
-			    "reference is ignored.\n"),
+			    "%lld (0x%llx), was previously referenced so "
+			    "the invalid reference is ignored.\n"),
 			  btype, (unsigned long long)block,
 			  (unsigned long long)block,
 			  (unsigned long long)ip->i_di.di_num.no_addr,
@@ -897,13 +898,13 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
 	long *bad_pointers = (long *)private;
 	uint8_t q;
 
-	if (gfs2_check_range(ip->i_sbd, block) != 0) {
+	if (!valid_block(ip->i_sbd, block) != 0) {
 		(*bad_pointers)++;
-		log_debug( _("Bad %s block pointer (out of range #%ld) "
-			     "found in inode %lld (0x%llx).\n"), btype,
-			   *bad_pointers,
-			   (unsigned long long)ip->i_di.di_num.no_addr,
-			   (unsigned long long)ip->i_di.di_num.no_addr);
+		log_info( _("Bad %s block pointer (invalid or out of range "
+			    "#%ld) found in inode %lld (0x%llx).\n"),
+			  btype, *bad_pointers,
+			  (unsigned long long)ip->i_di.di_num.no_addr,
+			  (unsigned long long)ip->i_di.di_num.no_addr);
 		if ((*bad_pointers) <= BAD_POINTER_TOLERANCE)
 			return ENOENT;
 		else
@@ -913,11 +914,12 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
 	q = block_type(block);
 	if (q != gfs2_block_free) {
 		(*bad_pointers)++;
-		log_debug( _("Duplicated %s block pointer (violation #%ld) "
-			     "found in inode %lld (0x%llx).\n"), btype,
-			   *bad_pointers,
-			   (unsigned long long)ip->i_di.di_num.no_addr,
-			   (unsigned long long)ip->i_di.di_num.no_addr);
+		log_info( _("Duplicated %s block pointer (violation %ld, block"
+			    " %lld (0x%llx)) found in inode %lld (0x%llx).\n"),
+			  btype, *bad_pointers,
+			  (unsigned long long)block, (unsigned long long)block,
+			  (unsigned long long)ip->i_di.di_num.no_addr,
+			  (unsigned long long)ip->i_di.di_num.no_addr);
 		if ((*bad_pointers) <= BAD_POINTER_TOLERANCE)
 			return ENOENT;
 		else
diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c
index 6b7bc41..bbf33d2 100644
--- a/gfs2/fsck/pass1b.c
+++ b/gfs2/fsck/pass1b.c
@@ -205,7 +205,7 @@ static int clear_dup_metalist(struct gfs2_inode *ip, uint64_t block,
 	struct dup_handler *dh = (struct dup_handler *) private;
 	struct duptree *d;
 
-	if (gfs2_check_range(ip->i_sbd, block) != 0)
+	if (!valid_block(ip->i_sbd, block) != 0)
 		return 0;
 
 	/* This gets tricky. We're traversing a metadata tree trying to
diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c
index 0fbe0ce..209c32d 100644
--- a/gfs2/fsck/pass1c.c
+++ b/gfs2/fsck/pass1c.c
@@ -78,10 +78,10 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block,
 	uint8_t q;
 	struct gfs2_buffer_head *indir_bh = NULL;
 
-	if (gfs2_check_range(sdp, block)) {
+	if (!valid_block(sdp, block)) {
 		log_err( _("Extended attributes indirect block #%llu"
 			" (0x%llx) for inode #%llu"
-			" (0x%llx) out of range...removing\n"),
+			" (0x%llx) is invalid...removing\n"),
 			(unsigned long long)block,
 			(unsigned long long)block,
 			(unsigned long long)ip->i_di.di_num.no_addr,
@@ -92,7 +92,7 @@ static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block,
 	if (q != gfs2_indir_blk) {
 		log_err( _("Extended attributes indirect block #%llu"
 			" (0x%llx) for inode #%llu"
-			" (0x%llx) invalid.\n"),
+			" (0x%llx) is invalid.\n"),
 			(unsigned long long)block,
 			(unsigned long long)block,
 			(unsigned long long)ip->i_di.di_num.no_addr,
@@ -113,9 +113,9 @@ static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block,
 	struct gfs2_sbd *sdp = ip->i_sbd;
 	uint8_t q;
 
-	if (gfs2_check_range(sdp, block)) {
+	if (!valid_block(sdp, block)) {
 		log_err( _("Extended attributes block for inode #%llu"
-			" (0x%llx) out of range.\n"),
+			" (0x%llx) is invalid.\n"),
 			(unsigned long long)ip->i_di.di_num.no_addr,
 			(unsigned long long)ip->i_di.di_num.no_addr);
 		return ask_remove_eattr(ip);
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 573ed30..72bd107 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -206,9 +206,9 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
 	else
 		strncpy(tmp_name, filename, MAX_FILENAME - 1);
 
-	if (gfs2_check_range(ip->i_sbd, entryblock)) {
+	if (!valid_block(ip->i_sbd, entryblock)) {
 		log_err( _("Block # referenced by directory entry %s in inode "
-			   "%lld (0x%llx) is out of range\n"),
+			   "%lld (0x%llx) is invalid\n"),
 			 tmp_name, (unsigned long long)ip->i_di.di_num.no_addr,
 			 (unsigned long long)ip->i_di.di_num.no_addr);
 		if (query( _("Clear directory entry to out of range block? "
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 1dd49b1..c91e4be 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -710,7 +710,7 @@ static int expect_rindex_sanity(struct gfs2_sbd *sdp, osi_list_t *ret_list,
 	osi_list_t *tmp;
 	struct rgrp_list *exp, *rgd; /* expected, actual */
 
-	*num_rgs = sdp->md.riinode->i_di.di_size / sizeof(struct gfs2_rindex);
+	*num_rgs = sdp->md.riinode->i_di.di_size / sizeof(struct gfs2_rindex) ;
 	osi_list_init(ret_list);
 	for (tmp = sdp->rglist.next; tmp != &sdp->rglist; tmp = tmp->next) {
 		rgd = osi_list_entry(tmp, struct rgrp_list, list);
diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
index d496833..706a386 100644
--- a/gfs2/fsck/util.c
+++ b/gfs2/fsck/util.c
@@ -204,7 +204,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
 	struct inode_with_dups *id, *found_id;
 	struct duptree *dt;
 
-	if (gfs2_check_range(ip->i_sbd, block) != 0)
+	if (!valid_block(ip->i_sbd, block) != 0)
 		return 0;
 	/* If this is not the first reference (i.e. all calls from pass1) we
 	   need to create the duplicate reference. If this is pass1b, we want
diff --git a/gfs2/libgfs2/fs_bits.c b/gfs2/libgfs2/fs_bits.c
index 97172df..9f71471 100644
--- a/gfs2/libgfs2/fs_bits.c
+++ b/gfs2/libgfs2/fs_bits.c
@@ -145,7 +145,24 @@ int gfs2_check_range(struct gfs2_sbd *sdp, uint64_t blkno)
 }
 
 /*
- * fs_set_bitmap
+ * valid_block - check if blkno is valid and not part of our rgrps or bitmaps
+ * @sdp: super block
+ * @blkno: block number
+ *
+ * Returns: 1 if ok, 0 if out of bounds
+ */
+int valid_block(struct gfs2_sbd *sdp, uint64_t blkno)
+{
+	if((blkno > sdp->fssize) || (blkno <= sdp->sb_addr))
+		return 0;
+	/* Check if the block is one of our rgrp or bitmap blocks */
+	if (gfs2_get_bitmap(sdp, blkno, NULL) < 0)
+		return 0;
+	return 1;
+}
+
+/*
+ * gfs2_set_bitmap
  * @sdp: super block
  * @blkno: block number relative to file system
  * @state: one of three possible states
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 254a8e6..140a7a0 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -410,6 +410,7 @@ extern uint32_t gfs2_blkalloc_internal(struct rgrp_list *rgd, uint32_t goal,
 extern int gfs2_check_range(struct gfs2_sbd *sdp, uint64_t blkno);
 
 /* functions with blk #'s that are file system relative */
+extern int valid_block(struct gfs2_sbd *sdp, uint64_t blkno);
 extern int gfs2_get_bitmap(struct gfs2_sbd *sdp, uint64_t blkno,
 			   struct rgrp_list *rgd);
 extern int gfs2_set_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, int state);


More information about the cluster-commits mailing list