Blob Blame History Raw
commit 5cf083a5c231bfe0950a276aff9249dd63f708d3
Author: Andrew Price <anprice@redhat.com>
Date:   Fri Oct 18 19:16:08 2019 +0100

    fsck.gfs2: Retain context for indirect pointers in ->check_metalist
    
    When the pass->check_metalist() functions are called, the pointer is
    thrown away and the function is just called with a block address,
    meaning that ->check_metalist() is unable to fix the pointer itself if
    it deems the block pointed to is garbage.
    
    Add an iptr structure which collects together the inode, the metadata
    block buffer and the offset of the pointer in that buffer. The iptr is
    now passed to ->check_metadata instead of the separate inode and block
    address arguments.
    
    Resolves: rhbz#1487726
    
    Signed-off-by: Andrew Price <anprice@redhat.com>

diff --git a/gfs2/fsck/afterpass1_common.c b/gfs2/fsck/afterpass1_common.c
index b7476408..55a6c76b 100644
--- a/gfs2/fsck/afterpass1_common.c
+++ b/gfs2/fsck/afterpass1_common.c
@@ -175,10 +175,12 @@ int remove_dentry_from_dir(struct gfs2_sbd *sdp, uint64_t dir,
 	return error;
 }
 
-int delete_metadata(struct gfs2_inode *ip, uint64_t block,
-		    struct gfs2_buffer_head **bh, int h, int *is_valid,
+int delete_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, int *is_valid,
 		    int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*is_valid = 1;
 	*was_duplicate = 0;
 	return delete_block_if_notdup(ip, block, bh, _("metadata"),
diff --git a/gfs2/fsck/afterpass1_common.h b/gfs2/fsck/afterpass1_common.h
index 829828f7..74b913f3 100644
--- a/gfs2/fsck/afterpass1_common.h
+++ b/gfs2/fsck/afterpass1_common.h
@@ -2,9 +2,9 @@
 #define _AFTERPASS1_H
 
 #include "util.h"
+#include "metawalk.h"
 
-extern int delete_metadata(struct gfs2_inode *ip, uint64_t block,
-			   struct gfs2_buffer_head **bh, int h, int *is_valid,
+extern int delete_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h, int *is_valid,
 			   int *was_duplicate, void *private);
 extern int delete_leaf(struct gfs2_inode *ip, uint64_t block, void *private);
 extern int delete_data(struct gfs2_inode *ip, uint64_t metablock,
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 3acbb5d7..614c01ac 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -636,11 +636,11 @@ static int rangecheck_jblock(struct gfs2_inode *ip, uint64_t block)
 	return meta_is_good;
 }
 
-static int rangecheck_jmeta(struct gfs2_inode *ip, uint64_t block,
-			       struct gfs2_buffer_head **bh, int h,
-			       int *is_valid, int *was_duplicate,
-			       void *private)
+static int rangecheck_jmeta(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                            int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
 	int rc;
 
 	*bh = NULL;
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index dbe9f1f6..d91ed63a 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1207,9 +1207,11 @@ static void file_ra(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 			      extlen * sdp->bsize, POSIX_FADV_WILLNEED);
 }
 
-static int do_check_metalist(struct gfs2_inode *ip, uint64_t block, int height,
-                             struct gfs2_buffer_head **bhp, struct metawalk_fxns *pass)
+static int do_check_metalist(struct iptr iptr, int height, struct gfs2_buffer_head **bhp,
+                             struct metawalk_fxns *pass)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
 	int was_duplicate = 0;
 	int is_valid = 1;
 	int error;
@@ -1217,7 +1219,7 @@ static int do_check_metalist(struct gfs2_inode *ip, uint64_t block, int height,
 	if (pass->check_metalist == NULL)
 		return 0;
 
-	error = pass->check_metalist(ip, block, bhp, height, &is_valid,
+	error = pass->check_metalist(iptr, bhp, height, &is_valid,
 				     &was_duplicate, pass->private);
 	if (error == meta_error) {
 		stack;
@@ -1267,10 +1269,11 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
 				    struct metawalk_fxns *pass)
 {
 	uint32_t height = ip->i_di.di_height;
-	struct gfs2_buffer_head *bh, *nbh, *metabh = ip->i_bh;
+	struct gfs2_buffer_head *metabh = ip->i_bh;
 	osi_list_t *prev_list, *cur_list, *tmp;
+	struct iptr iptr = { .ipt_ip = ip, 0};
 	int h, head_size, iblk_type;
-	uint64_t *ptr, block, *undoptr;
+	uint64_t *undoptr;
 	int maxptrs;
 	int error;
 
@@ -1310,39 +1313,37 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
 		prev_list = &mlp[h - 1];
 		cur_list = &mlp[h];
 
-		for (tmp = prev_list->next; tmp != prev_list; tmp = tmp->next){
-			bh = osi_list_entry(tmp, struct gfs2_buffer_head,
-					    b_altlist);
-			if (gfs2_check_meta(bh->b_data, iblk_type)) {
+		for (tmp = prev_list->next; tmp != prev_list; tmp = tmp->next) {
+			iptr.ipt_off = head_size;
+			iptr.ipt_bh = osi_list_entry(tmp, struct gfs2_buffer_head, b_altlist);
+
+			if (gfs2_check_meta(iptr_buf(iptr), iblk_type)) {
 				if (pass->invalid_meta_is_fatal)
 					return meta_error;
 
 				continue;
 			}
-
 			if (pass->readahead)
-				file_ra(ip, bh, head_size, maxptrs, h);
+				file_ra(ip, iptr.ipt_bh, head_size, maxptrs, h);
+
 			/* Now check the metadata itself */
-			for (ptr = (uint64_t *)(bh->b_data + head_size);
-			     (char *)ptr < (bh->b_data + ip->i_sbd->bsize);
-			     ptr++) {
+			for (; iptr.ipt_off < ip->i_sbd->bsize; iptr.ipt_off += sizeof(uint64_t)) {
+				struct gfs2_buffer_head *nbh = NULL;
+
 				if (skip_this_pass || fsck_abort) {
 					free_metalist(ip, mlp);
 					return meta_is_good;
 				}
-				nbh = NULL;
-
-				if (!*ptr)
+				if (!iptr_block(iptr))
 					continue;
 
-				block = be64_to_cpu(*ptr);
-				error = do_check_metalist(ip, block, h, &nbh, pass);
+				error = do_check_metalist(iptr, h, &nbh, pass);
 				if (error == meta_error || error == meta_skip_further)
 					goto error_undo;
 				if (error == meta_skip_one)
 					continue;
 				if (!nbh)
-					nbh = bread(ip->i_sbd, block);
+					nbh = bread(ip->i_sbd, iptr_block(iptr));
 				osi_list_add_prev(&nbh->b_altlist, cur_list);
 			} /* for all data on the indirect block */
 		} /* for blocks at that height */
@@ -1353,16 +1354,16 @@ error_undo: /* undo what we've done so far for this block */
 	if (pass->undo_check_meta == NULL)
 		return error;
 
-	log_info(_("Undoing the work we did before the error on block %llu "
-		   "(0x%llx).\n"), (unsigned long long)bh->b_blocknr,
-		 (unsigned long long)bh->b_blocknr);
-	for (undoptr = (uint64_t *)(bh->b_data + head_size); undoptr < ptr &&
-		     (char *)undoptr < (bh->b_data + ip->i_sbd->bsize);
+	log_info(_("Undoing the work we did before the error on block %"PRIu64" (0x%"PRIx64").\n"),
+	         iptr.ipt_bh->b_blocknr, iptr.ipt_bh->b_blocknr);
+	for (undoptr = (uint64_t *)(iptr_buf(iptr) + head_size);
+	     undoptr < iptr_ptr(iptr) && undoptr < iptr_endptr(iptr);
 	     undoptr++) {
-		if (!*undoptr)
+		uint64_t block = be64_to_cpu(*undoptr);
+
+		if (block == 0)
 			continue;
 
-		block = be64_to_cpu(*undoptr);
 		pass->undo_check_meta(ip, block, h, pass->private);
 	}
 	return error;
diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
index b5a037a3..592479df 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -42,6 +42,17 @@ enum meta_check_rc {
 	meta_skip_one = 2,
 };
 
+struct iptr {
+	struct gfs2_inode *ipt_ip;
+	struct gfs2_buffer_head *ipt_bh;
+	unsigned ipt_off;
+};
+
+#define iptr_ptr(i) ((uint64_t *)(i.ipt_bh->b_data + i.ipt_off))
+#define iptr_block(i) be64_to_cpu(*iptr_ptr(i))
+#define iptr_endptr(i) ((uint64_t *)(iptr.ipt_bh->b_data + i.ipt_ip->i_sbd->bsize))
+#define iptr_buf(i) (i.ipt_bh->b_data)
+
 /* metawalk_fxns: function pointers to check various parts of the fs
  *
  * The functions should return -1 on fatal errors, 1 if the block
@@ -66,7 +77,7 @@ struct metawalk_fxns {
 	int (*check_leaf) (struct gfs2_inode *ip, uint64_t block,
 			   void *private);
 	/* parameters to the check_metalist sub-functions:
-	   ip: incore inode pointer
+	   iptr: reference to the inode and its indirect pointer that we're analyzing
 	   block: block number of the metadata block to be checked
 	   bh: buffer_head to be returned
 	   h: height
@@ -79,7 +90,7 @@ struct metawalk_fxns {
 	   returns: 0 - everything is good, but there may be duplicates
 	            1 - skip further processing
 	*/
-	int (*check_metalist) (struct gfs2_inode *ip, uint64_t block,
+	int (*check_metalist) (struct iptr iptr,
 			       struct gfs2_buffer_head **bh, int h,
 			       int *is_valid, int *was_duplicate,
 			       void *private);
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index f03078e9..66cf6dc6 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -39,9 +39,8 @@ struct block_count {
 };
 
 static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private);
-static int pass1_check_metalist(struct gfs2_inode *ip, uint64_t block,
-			  struct gfs2_buffer_head **bh, int h, int *is_valid,
-			  int *was_duplicate, void *private);
+static int pass1_check_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                                int *is_valid, int *was_duplicate, void *private);
 static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block,
 			       int h, void *private);
 static int pass1_check_data(struct gfs2_inode *ip, uint64_t metablock,
@@ -69,10 +68,8 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, int i,
 				     void *private);
 static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers,
 			      int leaf_pointer_errors, void *private);
-static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block,
-			       struct gfs2_buffer_head **bh, int h,
-			       int *is_valid, int *was_duplicate,
-			       void *private);
+static int invalidate_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                               int *is_valid, int *was_duplicate, void *private);
 static int invalidate_leaf(struct gfs2_inode *ip, uint64_t block,
 			   void *private);
 static int invalidate_data(struct gfs2_inode *ip, uint64_t metablock,
@@ -223,12 +220,12 @@ struct metawalk_fxns invalidate_fxns = {
  * marked "in use" by the bitmap.  You don't want root's indirect blocks
  * deleted, do you? Or worse, reused for lost+found.
  */
-static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block,
-				struct gfs2_buffer_head **bh, int h,
-				int *is_valid, int *was_duplicate,
-				void *private)
+static int resuscitate_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                                int *is_valid, int *was_duplicate, void *private)
 {
 	struct block_count *bc = (struct block_count *)private;
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
 
 	*is_valid = 1;
 	*was_duplicate = 0;
@@ -344,15 +341,16 @@ static int p1check_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
 	return 0;
 }
 
-static int pass1_check_metalist(struct gfs2_inode *ip, uint64_t block,
-			  struct gfs2_buffer_head **bh, int h, int *is_valid,
-			  int *was_duplicate, void *private)
+static int pass1_check_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                                int *is_valid, int *was_duplicate, void *private)
 {
-	int q;
-	int iblk_type;
-	struct gfs2_buffer_head *nbh;
 	struct block_count *bc = (struct block_count *)private;
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+	struct gfs2_buffer_head *nbh;
 	const char *blktypedesc;
+	int iblk_type;
+	int q;
 
 	*bh = NULL;
 
@@ -1111,11 +1109,12 @@ static int mark_block_invalid(struct gfs2_inode *ip, uint64_t block,
 	return meta_is_good;
 }
 
-static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block,
-			       struct gfs2_buffer_head **bh, int h,
-			       int *is_valid, int *was_duplicate,
-			       void *private)
+static int invalidate_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                               int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*is_valid = 1;
 	*was_duplicate = 0;
 	return mark_block_invalid(ip, block, ref_as_meta, _("metadata"),
@@ -1214,11 +1213,12 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
 	return meta_is_good;
 }
 
-static int rangecheck_metadata(struct gfs2_inode *ip, uint64_t block,
-			       struct gfs2_buffer_head **bh, int h,
-			       int *is_valid, int *was_duplicate,
-			       void *private)
+static int rangecheck_metadata(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                               int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*is_valid = 1;
 	*was_duplicate = 0;
 	return rangecheck_block(ip, block, bh, btype_meta, private);
@@ -1317,12 +1317,13 @@ static int set_ip_blockmap(struct gfs2_inode *ip)
 	return 0;
 }
 
-static int alloc_metalist(struct gfs2_inode *ip, uint64_t block,
-			  struct gfs2_buffer_head **bh, int h, int *is_valid,
-			  int *was_duplicate, void *private)
+static int alloc_metalist(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                          int *is_valid, int *was_duplicate, void *private)
 {
-	int q;
 	const char *desc = (const char *)private;
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+	int q;
 
 	/* No need to range_check here--if it was added, it's in range. */
 	/* We can't check the bitmap here because this function is called
diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c
index 62686fee..350902b4 100644
--- a/gfs2/fsck/pass1b.c
+++ b/gfs2/fsck/pass1b.c
@@ -69,9 +69,8 @@ static void log_inode_reference(struct duptree *dt, osi_list_t *tmp, int inval)
 		  (unsigned long long)dt->block, reftypestring);
 }
 
-static int findref_meta(struct gfs2_inode *ip, uint64_t block,
-			struct gfs2_buffer_head **bh, int h,
-			int *is_valid, int *was_duplicate, void *private)
+static int findref_meta(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                        int *is_valid, int *was_duplicate, void *private)
 {
 	*is_valid = 1;
 	*was_duplicate = 0;
@@ -393,10 +392,12 @@ static void resolve_dup_references(struct gfs2_sbd *sdp, struct duptree *dt,
 	return;
 }
 
-static int clone_check_meta(struct gfs2_inode *ip, uint64_t block,
-			    struct gfs2_buffer_head **bh, int h,
-			    int *is_valid, int *was_duplicate, void *private)
+static int clone_check_meta(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                            int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*was_duplicate = 0;
 	*is_valid = 1;
 	*bh = bread(ip->i_sbd, block);
@@ -788,11 +789,12 @@ static int check_leaf_refs(struct gfs2_inode *ip, uint64_t block,
 	return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID);
 }
 
-static int check_metalist_refs(struct gfs2_inode *ip, uint64_t block,
-			       struct gfs2_buffer_head **bh, int h,
-			       int *is_valid, int *was_duplicate,
-			       void *private)
+static int check_metalist_refs(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                               int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*was_duplicate = 0;
 	*is_valid = 1;
 	return add_duplicate_ref(ip, block, ref_as_meta, 1, INODE_VALID);
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index d10b9089..d80f3063 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1873,10 +1873,12 @@ struct metawalk_fxns pass2_fxns = {
 	.repair_leaf = pass2_repair_leaf,
 };
 
-static int check_metalist_qc(struct gfs2_inode *ip, uint64_t block,
-			     struct gfs2_buffer_head **bh, int h,
-			     int *is_valid, int *was_duplicate, void *private)
+static int check_metalist_qc(struct iptr iptr, struct gfs2_buffer_head **bh, int h,
+                             int *is_valid, int *was_duplicate, void *private)
 {
+	struct gfs2_inode *ip = iptr.ipt_ip;
+	uint64_t block = iptr_block(iptr);
+
 	*was_duplicate = 0;
 	*is_valid = 1;
 	*bh = bread(ip->i_sbd, block);