cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: rpeterso@redhat.com <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 47/66] fsck.gfs2: Bad extended attributes not deleted properly
Date: Fri, 20 Jan 2012 09:10:28 -0600	[thread overview]
Message-ID: <1327072247-26275-48-git-send-email-rpeterso@redhat.com> (raw)
In-Reply-To: <1327072247-26275-1-git-send-email-rpeterso@redhat.com>

From: Bob Peterson <rpeterso@redhat.com>

This patch fixes a couple of problems fsck.gfs2 when it discovered
corrupt extended attributes and tried to delete them.  First, in
check_eattr_entries it wasn't reporting the corruption.  Second,
when the EA block was freed, it wasn't returning a proper return
code which caused fsck to not update the inode accordingly.
Third, it wasn't zeroing out the EA block address properly.
Fourth, I did a small amount of reformatting.

rhbz#675723
---
 gfs2/fsck/metawalk.c |   56 ++++++++++++++++++++++++++++++++++++++++---------
 gfs2/fsck/pass1.c    |    4 +--
 gfs2/fsck/pass1c.c   |    7 +++--
 gfs2/fsck/pass2.c    |    9 +++++--
 4 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 94b0148..db7168b 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -755,6 +755,13 @@ static int check_eattr_entries(struct gfs2_inode *ip,
 							      bh, ea_hdr,
 							      ea_hdr_prev,
 							      pass->private)) {
+					log_err(_("Bad extended attribute "
+						  "found at block %lld "
+						  "(0x%llx)"),
+						(unsigned long long)
+						be64_to_cpu(*ea_data_ptr),
+						(unsigned long long)
+						be64_to_cpu(*ea_data_ptr));
 					if (query( _("Repair the bad Extended "
 						     "Attribute? (y/n) "))) {
 						ea_hdr->ea_num_ptrs = i;
@@ -807,12 +814,14 @@ static int check_leaf_eattr(struct gfs2_inode *ip, uint64_t block,
 			    uint64_t parent, struct metawalk_fxns *pass)
 {
 	struct gfs2_buffer_head *bh = NULL;
-	int error = 0;
-
-	log_debug( _("Checking EA leaf block #%"PRIu64" (0x%" PRIx64 ").\n"),
-			  block, block);
 
 	if (pass->check_eattr_leaf) {
+		int error = 0;
+
+		log_debug( _("Checking EA leaf block #%llu (0x%llx).\n"),
+			   (unsigned long long)block,
+			   (unsigned long long)block);
+
 		error = pass->check_eattr_leaf(ip, block, parent, &bh,
 					       pass->private);
 		if (error < 0) {
@@ -886,13 +895,18 @@ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype)
 /**
  * free_block_if_notdup - free blocks associated with an inode, but if it's a
  *                        duplicate, just remove that designation instead.
- * Returns: 0 if the block was freed, 1 if a duplicate reference was removed
+ * Returns: 1 if the block was freed, 0 if a duplicate reference was removed
+ * Note: The return code is handled this way because there are places in
+ *       metawalk.c that assume "1" means "change was made" and "0" means
+ *       change was not made.
  */
 int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block,
 			 const char *btype)
 {
-	if (!find_remove_dup(ip, block, btype))
+	if (!find_remove_dup(ip, block, btype)) { /* not a dup */
 		fsck_blockmap_set(ip, block, btype, gfs2_block_free);
+		return 1;
+	}
 	return 0;
 }
 
@@ -1446,16 +1460,36 @@ int delete_data(struct gfs2_inode *ip, uint64_t block, void *private)
 int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
 		       struct gfs2_buffer_head **bh, void *private)
 {
-	return delete_block_if_notdup(ip, block, NULL,
-				      _("indirect extended attribute"),
-				      private);
+	int ret;
+
+	ret = delete_block_if_notdup(ip, block, NULL,
+				     _("indirect extended attribute"),
+				     private);
+	/* Even if it's a duplicate reference, we want to eliminate the
+	   reference itself, and adjust di_blocks accordingly. */
+	if (ip->i_di.di_eattr) {
+		ip->i_di.di_blocks--;
+		if (block == ip->i_di.di_eattr)
+			ip->i_di.di_eattr = 0;
+		bmodified(ip->i_bh);
+	}
+	return ret;
 }
 
 int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
 		      struct gfs2_buffer_head **bh, void *private)
 {
-	return delete_block_if_notdup(ip, block, NULL, _("extended attribute"),
-				      private);
+	int ret;
+
+	ret = delete_block_if_notdup(ip, block, NULL, _("extended attribute"),
+				     private);
+	if (ip->i_di.di_eattr) {
+		ip->i_di.di_blocks--;
+		if (block == ip->i_di.di_eattr)
+			ip->i_di.di_eattr = 0;
+		bmodified(ip->i_bh);
+	}
+	return ret;
 }
 
 static int alloc_metalist(struct gfs2_inode *ip, uint64_t block,
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 32ebfba..3e6b816 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -1181,9 +1181,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
 			  (unsigned long long)ip->i_di.di_num.no_addr,
 			  (unsigned long long)ip->i_di.di_num.no_addr);
 		check_metatree(ip, &invalidate_fxns);
-		if (fsck_blockmap_set(ip, block, _("invalid mode"),
-				      gfs2_inode_invalid))
-			goto bad_dinode;
+		check_inode_eattr(ip, &invalidate_fxns);
 		return 0;
 	} else if (error)
 		goto bad_dinode;
diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c
index 150dc2f..f7a7842 100644
--- a/gfs2/fsck/pass1c.c
+++ b/gfs2/fsck/pass1c.c
@@ -62,9 +62,10 @@ static int ask_remove_eattr(struct gfs2_inode *ip)
 		ip->i_di.di_eattr = 0;
 		bmodified(ip->i_bh);
 		log_err( _("Bad Extended Attribute removed.\n"));
-	} else
-		log_err( _("Bad Extended Attribute not removed.\n"));
-	return 1;
+		return 1;
+	}
+	log_err( _("Bad Extended Attribute not removed.\n"));
+	return 0;
 }
 
 static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block,
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 7e20315..e02fdd0 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -203,9 +203,9 @@ static int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr,
 				 struct gfs2_ea_header *ea_hdr_prev,
 				 void *private)
 {
-        uint64_t block = be64_to_cpu(*ea_data_ptr);
+	uint64_t block = be64_to_cpu(*ea_data_ptr);
 
-        return delete_metadata(ip, block, NULL, 0, private);
+	return delete_metadata(ip, block, NULL, 0, private);
 }
 
 struct metawalk_fxns pass2_fxns_delete = {
@@ -343,7 +343,10 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
 				entry_ip = ip;
 			else
 				entry_ip = fsck_load_inode(sdp, entryblock);
-			check_inode_eattr(entry_ip, &pass2_fxns_delete);
+			if (ip->i_di.di_eattr) {
+				check_inode_eattr(entry_ip,
+						  &pass2_fxns_delete);
+			}
 			check_metatree(entry_ip, &pass2_fxns_delete);
 			if (entry_ip != ip)
 				fsck_inode_put(&entry_ip);
-- 
1.7.7.5



  parent reply	other threads:[~2012-01-20 15:10 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-20 15:09 [Cluster-devel] [PATCH 00/66] fsck.gfs2: add ability to fix GFS (gfs1) file systems rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 01/66] fsck.gfs2: Make functions use sdp rather than sbp rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 02/66] fsck.gfs2: Change "if(" to "if (" rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 03/66] libgfs1: Add gfs1 variable to superblock structure rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 04/66] libgfs2: Make check_sb and read_sb operate on gfs1 rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 05/66] libgfs2: move gfs1 structures to libgfs2 rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 06/66] fsck.gfs2: Check for blocks wrongly inside resource groups rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 07/66] fsck.gfs2: Rename check_leaf to check_ealeaf_block rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 08/66] fsck.gfs2: fsck.gfs2: Delete vestigial buffer_head in check_leaf rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 09/66] fsck.gfs2: fsck.gfs2: Rename nlink functions to be intuitive rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 10/66] fsck.gfs2: fsck.gfs2: Sync di_nlink adding links for lost+found rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 11/66] fsck.gfs2: fsck.gfs2: Make dir entry count 32 bits rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 12/66] fsck.gfs2: get rid of triple negative logic rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 13/66] dirent_repair needs to mark the buffer as modified rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 14/66] fsck.gfs2: fsck.gfs2: Ask to reclaim unlinked meta per-rgrp only rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 15/66] fsck.gfs2: fsck.gfs2: Refactor add_dotdot function in lost+found rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 16/66] libgfs2: libgfs2: Use __FUNCTION__ rather than __FILE__ rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 17/66] fsck.gfs2: fsck.gfs2: Don't stop invalidating blocks on invalid rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 18/66] fsck.gfs2: fsck.gfs2: Find and clear duplicate leaf blocks refs rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 19/66] fsck.gfs2: fsck.gfs2: Move check_num_ptrs from metawalk to pass1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 20/66] fsck.gfs2: fsck.gfs2: Duplicate ref processing for leaf blocks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 21/66] fsck.gfs2: fsck.gfs2: split check_leaf_blks to be more readable rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 22/66] fsck.gfs2: Shorten output rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 23/66] fsck.gfs2: Make output messages more sensible rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 24/66] fsck.gfs pass2: Refactor function set_dotdot_dir rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 25/66] fsck.gfs2 pass2: Delete extended attributes with inode rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 26/66] fsck.gfs2 pass2: Don't delete invalid inode metadata rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 27/66] fsck.gfs2 pass3: Refactor mark_and_return_parent rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 28/66] fsck.gfs2: misc cosmetic changes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 29/66] fsck.gfs2: Don't use old_leaf if it was a duplicate rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 30/66] fsck.gfs2: Add find_remove_dup, free_block_if_notdup rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 31/66] fsck.gfs2: don't free prev rgrp list repairing rgrps rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 32/66] libgfs2: eliminate gfs1_readi in favor of gfs2_readi rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 33/66] libgfs2: Mark buffer modified adding a new GFS1 block rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 34/66] libgfs2: Use dinode buffer to map gfs1 dinode blocks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 35/66] libgfs2: move block_map functions to fsck.gfs2 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 36/66] libgfs2: eliminate gfs1_rindex_read rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 37/66] libgfs2: combine ri_update and gfs1_ri_update rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 38/66] libgfs2: combine gfs_inode_read and gfs_inode_get rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 39/66] libgfs2: move gfs1 functions from edit to libgfs2 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 40/66] gfs2_edit savemeta: save_inode_data backward for gfs1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 41/66] libgfs2: expand capabilities to operate on gfs1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 42/66] fsck.gfs2: Combine block and char device inode types rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 43/66] fsck.gfs2: four-step duplicate elimination process rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 44/66] fsck.gfs2: Add ability to check gfs1 file systems rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 45/66] fsck.gfs2: Remove bad inodes from duplicate tree rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 46/66] fsck.gfs2: Handle duplicate reference to dinode blocks rpeterso
2012-01-20 15:10 ` rpeterso [this message]
2012-01-20 15:10 ` [Cluster-devel] [PATCH 48/66] libgfs2: Make rebuild functions not re-read ip rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 49/66] fsck.gfs2: Shorten debug output rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 50/66] fsck.gfs2: Increment link count reporting wrong dinode rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 51/66] fsck.gfs2: system dinodes take priority over user dinodes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 52/66] fsck.gfs2: Recognize partially gfs2-converted dinodes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 53/66] fsck.gfs2: Print step 2 duplicate debug msg first rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 54/66] fsck.gfs2: pass1c counts percentage backward rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 55/66] fsck.gfs2: Speed up rangecheck functions rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 56/66] libgfs2: Make in-core rgrps use rbtree rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 57/66] fsck.gfs2: Fix memory leaks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 58/66] Change man pages and gfs2_convert messages to include GFS rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 59/66] gfs2_edit: Fix memory leaks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 60/66] fsck.gfs2: Journals not properly checked rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 61/66] fsck.gfs2: Rearrange block types to group all inode types rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 62/66] fsck.gfs2: Fix initialization error return codes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 63/66] fsck.gfs2: Don't use strerror for libgfs2 errors rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 64/66] fsck.gfs2: Fix memory leak in initialize.c rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 65/66] fsck.gfs2: Add return code checks and initializations rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 66/66] libgfs2: Fix null pointer dereference in linked_leaf_search rpeterso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1327072247-26275-48-git-send-email-rpeterso@redhat.com \
    --to=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).