fsck.gfs2: Fix reference to uninitialized variable

This patch checks directory hash tables for multiple occurrences of the same leaf block. When duplicates are encountered, it checks to see which reference is appropriate for the hash table location based on the leaf block's hash value. The appropriate reference is kept and all duplicates are replaced by newly created leaf blocks.

rhbz#984085

---
  gfs2/fsck/pass2.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 108 insertions(+)

gfs2/fsck/pass2.c +44 -0

index a40a4a7..9f3e5c1 100644
@@ -1278,6 +1278,110 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
  	return changes;
  }
 
+/* check_hash_tbl_dups - check for the same leaf in multiple places */
+static int check_hash_tbl_dups(struct gfs2_inode *ip, uint64_t *tbl,
+			       unsigned hsize, int lindex, int len)
+{
+	int l, len2;
+	uint64_t leafblk, leaf_no;
+	struct gfs2_buffer_head *lbh;
+	struct gfs2_leaf leaf;
+	struct gfs2_dirent dentry, *de;
+	int hash_index; /* index into the hash table based on the hash */
+
+	leafblk = be64_to_cpu(tbl[lindex]);
+	for (l = 0; l < hsize; l++) {
+		if (l == lindex) { /* skip the valid reference */
+			l += len - 1;
+			continue;
+		}
+		if (be64_to_cpu(tbl[l]) != leafblk)
+			continue;
+
+		for (len2 = 0; l + len2 < hsize; len2++) {
+			if (l + len2 == lindex)
+				break;
+			if (be64_to_cpu(tbl[l + len2]) != leafblk)
+				break;
+		}
+		log_err(_("Dinode %llu (0x%llx) has duplicate leaf pointers "
+			  "to block %llu (0x%llx) at offsets %u (0x%x) "
+			  "(for 0x%x) and %u (0x%x) (for 0x%x)\n"),
+			(unsigned long long)ip->i_di.di_num.no_addr,
+			(unsigned long long)ip->i_di.di_num.no_addr,
+			(unsigned long long)leafblk,
+			(unsigned long long)leafblk, lindex, lindex, len,
+			l, l, len2);
+
+		/* See which set of references is valid: the one passed in
+		   or the duplicate we found. */
+		memset(&leaf, 0, sizeof(leaf));
+		leaf_no = leafblk;
+		if (!valid_block(ip->i_sbd, leaf_no)) /* Checked later */
+			continue;
+
+		lbh = bread(ip->i_sbd, leafblk);
+		if (gfs2_check_meta(lbh, GFS2_METATYPE_LF)) { /* Chked later */

gfs2/fsck/pass2.c +15 -0

+				return 0;
+			}
+			/* Adjust the ondisk block count. The original value
+			   may have been correct without the duplicates but
+			   pass1 would have counted them and adjusted the
+			   count to include them. So we must subtract them. */
+			ip->i_di.di_blocks--;
+			bmodified(ip->i_bh);
+			pad_with_leafblks(ip, tbl, l, len2);
+		} else {
+			log_debug(_("Hash index 0x%x is the proper "
+				    "reference to leaf 0x%llx.\n"),
+				  l, (unsigned long long)leafblk);
+		}
+		/* Check the original ref: both references might be bad.

gfs2/fsck/pass2.c +21 -0

+				return 0;
+			}
+			ip->i_di.di_blocks--;
+			bmodified(ip->i_bh);
+			pad_with_leafblks(ip, tbl, lindex, len);
+			/* At this point we know both copies are bad, so we
+			   return to start fresh */
+			return -EFAULT;
+		} else {
+			log_debug(_("Hash index 0x%x is the proper "
+				    "reference to leaf 0x%llx.\n"),
+				  lindex, (unsigned long long)leafblk);
+		}
+	}
+	return 0;
+}
+
  /* check_hash_tbl - check that the hash table is sane
   *
   * We've got to make sure the hash table is sane. Each leaf needs to

@@ -1372,6 +1476,10 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl,
  			lindex += proper_len;
  			continue;
  		}
+
+		if (check_hash_tbl_dups(ip, tbl, hsize, lindex, len))
+			continue;
+		
  		/* Make sure they call on proper leaf-split boundaries. This
  		   is the calculation used by the kernel, and dir_split_leaf */
  		proper_start = (lindex & ~(proper_len - 1));

This patch initializes a variable so that it no longer references it uninitialized.

rhbz#984085

---
 gfs2/fsck/initialize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

gfs2/fsck/initialize.c +0 -0

index b01b240..936fd5e 100644

Quoted 13 lines · Bob Peterson

This patch initializes a variable so that it no longer references it uninitialized.

rhbz#984085

---
 gfs2/fsck/initialize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

gfs2/fsck/initialize.c +0 -0

index b01b240..936fd5e 100644

gfs2/fsck/initialize.c static int get_lockproto_table(struct gfs2_sbd *sdp) · line 836

Quoted 6 lines

gfs2/fsck/initialize.c +1 -1

@@ -832,7 +832,7 @@ static int get_lockproto_table(struct gfs2_sbd *sdp)
 {
 	FILE *fp;
 	char line[PATH_MAX];
-	char *cluname, *end;
+	char *cluname, *end = NULL;
 	const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";

Just spotted this reference to cluster.conf ^^ remember it doesn't exist anymore in the new era.

Fabio

As cluster.conf no longer exists we can't sniff the locking options from it when rebuilding the superblock and in any case we shouldn't assume that fsck.gfs2 is running on the cluster the volume belongs to.

This patch removes the get_lockproto_table function and instead sets the lock table name to a placeholder ("unknown") and sets lockproto to "lock_dlm". It warns the user at the end of the run that the locktable will need to be set before mounting.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/fsck/initialize.c | 57 ++++----------------------------------------------
 gfs2/fsck/main.c       |  4 ++++
 2 files changed, 8 insertions(+), 53 deletions(-)

gfs2/fsck/initialize.c +4 -53

index b01b240..869d2de 100644
@@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
 static uint64_t possible_root = HIGHEST_BLOCK;
 static struct master_dir fix_md;
 static unsigned long long blks_2free = 0;
+extern int sb_fixed;
 
 /**
  * block_mounters

@@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	return -1;
 }
 
-static int get_lockproto_table(struct gfs2_sbd *sdp)
-{
-	FILE *fp;
-	char line[PATH_MAX];
-	char *cluname, *end;
-	const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
-
-	memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
-	memset(sdp->locktable, 0, sizeof(sdp->locktable));
-	fp = fopen(cfgfile, "rt");
-	if (!fp) {
-		/* no cluster.conf; must be a stand-alone file system */
-		strcpy(sdp->lockproto, "lock_nolock");
-		log_warn(_("Lock protocol determined to be: lock_nolock\n"));
-		log_warn(_("Stand-alone file system: No need for a lock "
-			   "table.\n"));
-		return 0;
-	}
-	/* We found a cluster.conf so assume it's a clustered file system */
-	log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
-		   "\n"));
-	strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
-
-	while (fgets(line, sizeof(line) - 1, fp)) {
-		cluname = strstr(line,"<cluster name=");
-		if (cluname) {
-			cluname += 15;
-			end = strchr(cluname,'"');
-			if (end)
-				*end = '\0';
-			break;
-		}
-	}
-	if (cluname == NULL || end == NULL || end - cluname < 1) {
-		log_err(_("Error: Unable to determine cluster name from %s\n"),
-		                                                      cfgfile);
-	} else {
-		fsname = strrchr(opts.device, '/');
-		if (fsname)
-			fsname++;
-		else
-			fsname = "repaired";
-		snprintf(sdp->locktable, sizeof(sdp->locktable), "%.*s:%.16s",
-		         (int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
-		         cluname, fsname);
-		log_warn(_("Lock table determined to be: %s\n"),
-			 sdp->locktable);
-	}
-	fclose(fp);
-	return 0;
-}
-
 /**
  * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
  * A real dinode will be located at the block number in its no_addr.

@@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
 		}
 	}
 	/* Step 3 - Rebuild the lock protocol and file system table name */
-	get_lockproto_table(sdp);
+	strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
+	strcpy(sdp->locktable, "unknown");
 	if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
 		log_info(_("Found system master directory at: 0x%llx\n"),
 			 sdp->sd_sb.sb_master_dir.no_addr);

@@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
 		build_sb(sdp, uuid);
 		inode_put(&sdp->md.rooti);
 		inode_put(&sdp->master_dir);
+		sb_fixed = 1;
 	} else {
 		log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
 			   "without a valid superblock.\n"));

gfs2/fsck/main.c +4 -0

index 9c3b06d..f9e7166 100644
@@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
 struct osi_root inodetree = (struct osi_root) { NULL, };
 int dups_found = 0, dups_found_first = 0;
 struct gfs_sb *sbd1 = NULL;
+int sb_fixed = 0;
 
 /* This function is for libgfs2's sake.                                      */
 void print_it(const char *label, const char *fmt, const char *fmt2, ...)

@@ -315,6 +316,9 @@ int main(int argc, char **argv)
 		log_notice( _("Writing changes to disk\n"));
 	fsync(sdp->device_fd);
 	destroy(sdp);
+	if (sb_fixed)
+		log_warn(_("Superblock was reset. Use tunegfs2 to manually "
+		           "set lock table before mounting.\n"));
 	log_notice( _("gfs2_fsck complete\n"));
 
 	if (!error) {

As cluster.conf no longer exists we can't sniff the locking options from it when rebuilding the superblock and in any case we shouldn't assume that fsck.gfs2 is running on the cluster the volume belongs to.

This patch removes the get_lockproto_table function and instead sets the lock table name to a placeholder ("unknown") and sets lockproto to "lock_dlm". It warns the user at the end of the run that the locktable will need to be set before mounting.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/fsck/initialize.c | 57 ++++----------------------------------------------
 gfs2/fsck/main.c       |  4 ++++
 2 files changed, 8 insertions(+), 53 deletions(-)

gfs2/fsck/initialize.c +4 -53

index b01b240..869d2de 100644
@@ -33,6 +33,7 @@ static int was_mounted_ro = 0;
 static uint64_t possible_root = HIGHEST_BLOCK;
 static struct master_dir fix_md;
 static unsigned long long blks_2free = 0;
+extern int sb_fixed;
 
 /**
  * block_mounters

@@ -828,58 +829,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	return -1;
 }
 
-static int get_lockproto_table(struct gfs2_sbd *sdp)
-{
-	FILE *fp;
-	char line[PATH_MAX];
-	char *cluname, *end;
-	const char *fsname, *cfgfile = "/etc/cluster/cluster.conf";
-
-	memset(sdp->lockproto, 0, sizeof(sdp->lockproto));
-	memset(sdp->locktable, 0, sizeof(sdp->locktable));
-	fp = fopen(cfgfile, "rt");
-	if (!fp) {
-		/* no cluster.conf; must be a stand-alone file system */
-		strcpy(sdp->lockproto, "lock_nolock");
-		log_warn(_("Lock protocol determined to be: lock_nolock\n"));
-		log_warn(_("Stand-alone file system: No need for a lock "
-			   "table.\n"));
-		return 0;
-	}
-	/* We found a cluster.conf so assume it's a clustered file system */
-	log_warn(_("Lock protocol assumed to be: " GFS2_DEFAULT_LOCKPROTO
-		   "\n"));
-	strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
-
-	while (fgets(line, sizeof(line) - 1, fp)) {
-		cluname = strstr(line,"<cluster name=");
-		if (cluname) {
-			cluname += 15;
-			end = strchr(cluname,'"');
-			if (end)
-				*end = '\0';
-			break;
-		}
-	}
-	if (cluname == NULL || end == NULL || end - cluname < 1) {
-		log_err(_("Error: Unable to determine cluster name from %s\n"),
-		                                                      cfgfile);
-	} else {
-		fsname = strrchr(opts.device, '/');
-		if (fsname)
-			fsname++;
-		else
-			fsname = "repaired";
-		snprintf(sdp->locktable, sizeof(sdp->locktable), "%.*s:%.16s",
-		         (int)(sizeof(sdp->locktable) - strlen(fsname) - 2),
-		         cluname, fsname);
-		log_warn(_("Lock table determined to be: %s\n"),
-			 sdp->locktable);
-	}
-	fclose(fp);
-	return 0;
-}
-
 /**
  * is_journal_copy - Is this a "real" dinode or a copy inside a journal?
  * A real dinode will be located at the block number in its no_addr.

@@ -1256,7 +1205,8 @@ static int sb_repair(struct gfs2_sbd *sdp)
 		}
 	}
 	/* Step 3 - Rebuild the lock protocol and file system table name */
-	get_lockproto_table(sdp);
+	strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
+	strcpy(sdp->locktable, "unknown");
 	if (query(_("Okay to fix the GFS2 superblock? (y/n)"))) {
 		log_info(_("Found system master directory at: 0x%llx\n"),
 			 sdp->sd_sb.sb_master_dir.no_addr);

@@ -1280,6 +1230,7 @@ static int sb_repair(struct gfs2_sbd *sdp)
 		build_sb(sdp, uuid);
 		inode_put(&sdp->md.rooti);
 		inode_put(&sdp->master_dir);
+		sb_fixed = 1;
 	} else {
 		log_crit(_("GFS2 superblock not fixed; fsck cannot proceed "
 			   "without a valid superblock.\n"));

gfs2/fsck/main.c +4 -0

index 9c3b06d..f9e7166 100644
@@ -36,6 +36,7 @@ struct osi_root dirtree = (struct osi_root) { NULL, };
 struct osi_root inodetree = (struct osi_root) { NULL, };
 int dups_found = 0, dups_found_first = 0;
 struct gfs_sb *sbd1 = NULL;
+int sb_fixed = 0;
 
 /* This function is for libgfs2's sake.                                      */
 void print_it(const char *label, const char *fmt, const char *fmt2, ...)

@@ -315,6 +316,9 @@ int main(int argc, char **argv)
 		log_notice( _("Writing changes to disk\n"));
 	fsync(sdp->device_fd);
 	destroy(sdp);
+	if (sb_fixed)
+		log_warn(_("Superblock was reset. Use tunegfs2 to manually "
+		           "set lock table before mounting.\n"));
 	log_notice( _("gfs2_fsck complete\n"));
 
 	if (!error) {