From: rpeterso@redhat.com <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 35/42] fsck.gfs2 pass2: check leaf blocks when fixing hash table
Date: Mon, 8 Apr 2013 07:41:07 -0700 [thread overview]
Message-ID: <1365432074-17615-36-git-send-email-rpeterso@redhat.com> (raw)
In-Reply-To: <1365432074-17615-1-git-send-email-rpeterso@redhat.com>
From: Bob Peterson <rpeterso@redhat.com>
Before this patch, pass2 would attempt to fix the hash table without
first checking the basic integrity of the leaf blocks it was checking.
A misplaced leaf might have its entries relocated as a matter of course.
But if that leaf block had a problem, it could cause all kinds of
errors, including segfaults. This patch gives the hash table repair
function the ability to do basic integrity checks on the leaf block,
and perform repairs if necessary.
rhbz#902920
---
gfs2/fsck/pass2.c | 100 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 79 insertions(+), 21 deletions(-)
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 1e7f884..e38841e 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1050,6 +1050,66 @@ static int lost_leaf(struct gfs2_inode *ip, uint64_t *tbl, uint64_t leafno,
return 1;
}
+static int basic_check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
+ struct gfs2_dirent *prev_de,
+ struct gfs2_buffer_head *bh, char *filename,
+ uint32_t *count, int lindex, void *priv)
+{
+ uint8_t q = 0;
+ char tmp_name[MAX_FILENAME];
+ struct gfs2_inum entry;
+ struct dir_status *ds = (struct dir_status *) priv;
+ struct gfs2_dirent dentry, *de;
+ int error;
+
+ memset(&dentry, 0, sizeof(struct gfs2_dirent));
+ gfs2_dirent_in(&dentry, (char *)dent);
+ de = &dentry;
+
+ entry.no_addr = de->de_inum.no_addr;
+ entry.no_formal_ino = de->de_inum.no_formal_ino;
+
+ /* Start of checks */
+ memset(tmp_name, 0, MAX_FILENAME);
+ if (de->de_name_len < MAX_FILENAME)
+ strncpy(tmp_name, filename, de->de_name_len);
+ else
+ strncpy(tmp_name, filename, MAX_FILENAME - 1);
+
+ error = basic_dentry_checks(ip, dent, &entry, tmp_name, count, de,
+ ds, &q, bh);
+ if (error) {
+ dirent2_del(ip, bh, prev_de, dent);
+ log_err( _("Bad directory entry '%s' cleared.\n"), tmp_name);
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
+static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no,
+ int lindex, int ref_count, const char *msg,
+ void *private)
+{
+ return repair_leaf(ip, leaf_no, lindex, ref_count, msg);
+}
+
+/* The purpose of leafck_fxns is to provide a means for function fix_hashtable
+ * to do basic sanity checks on leaf blocks before manipulating them, for
+ * example, splitting them. If they're corrupt, splitting them or trying to
+ * move their contents can cause a segfault. We can't really use the standard
+ * pass2_fxns because that will do things we don't want. For example, it will
+ * find '.' and '..' and increment the directory link count, which would be
+ * done a second time when the dirent is really checked in pass2_fxns.
+ * We don't want it to do the "wrong leaf" thing, or set_parent_dir either.
+ * We just want a basic sanity check on pointers and lengths.
+ */
+struct metawalk_fxns leafck_fxns = {
+ .check_leaf_depth = check_leaf_depth,
+ .check_dentry = basic_check_dentry,
+ .repair_leaf = pass2_repair_leaf,
+};
+
/* fix_hashtable - fix a corrupt hash table
*
* The main intent of this function is to sort out hash table problems.
@@ -1079,10 +1139,11 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
int len, int *proper_len, int factor)
{
struct gfs2_buffer_head *lbh;
- struct gfs2_leaf *leaf;
+ struct gfs2_leaf leaf;
struct gfs2_dirent dentry, *de;
int changes = 0, error, i, extras, hash_index;
uint64_t new_leaf_blk;
+ uint64_t leaf_no;
uint32_t leaf_proper_start;
*proper_len = len;
@@ -1096,14 +1157,20 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
return 0;
}
+ memset(&leaf, 0, sizeof(leaf));
+ leaf_no = leafblk;
+ error = check_leaf(ip, lindex, &leafck_fxns, &leaf_no, &leaf, &len);
+ if (error) {
+ log_debug("Leaf repaired while fixing the hash table.\n");
+ error = 0;
+ }
lbh = bread(ip->i_sbd, leafblk);
- leaf = (struct gfs2_leaf *)lbh->b_data;
/* If the leaf's depth is out of range for this dinode, it's obviously
attached to the wrong dinode. Move the dirents to lost+found. */
- if (be16_to_cpu(leaf->lf_depth) > ip->i_di.di_depth) {
+ if (leaf.lf_depth > ip->i_di.di_depth) {
log_err(_("This leaf block's depth (%d) is too big for this "
"dinode's depth (%d)\n"),
- be16_to_cpu(leaf->lf_depth), ip->i_di.di_depth);
+ leaf.lf_depth, ip->i_di.di_depth);
error = lost_leaf(ip, tbl, leafblk, len, lindex, lbh);
brelse(lbh);
return error;
@@ -1129,7 +1196,7 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
}
/* Calculate the proper number of pointers based on the leaf depth. */
- *proper_len = 1 << (ip->i_di.di_depth - be16_to_cpu(leaf->lf_depth));
+ *proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth);
/* Look at the first dirent and check its hash value to see if it's
at the proper starting offset. */
@@ -1162,7 +1229,7 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
already at its maximum depth. */
if ((leaf_proper_start < proper_start) ||
((*proper_len > len || lindex > leaf_proper_start) &&
- be16_to_cpu(leaf->lf_depth) == ip->i_di.di_depth)) {
+ leaf.lf_depth == ip->i_di.di_depth)) {
log_err(_("Leaf block should start at 0x%x, but it appears at "
"0x%x in the hash table.\n"), leaf_proper_start,
proper_start);
@@ -1177,24 +1244,22 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
later than they should, we can split the leaf to give it a smaller
footprint in the hash table. */
if ((*proper_len > len || lindex > leaf_proper_start) &&
- ip->i_di.di_depth > be16_to_cpu(leaf->lf_depth)) {
+ ip->i_di.di_depth > leaf.lf_depth) {
log_err(_("For depth %d, length %d, the proper start is: "
"0x%x.\n"), factor, len, proper_start);
changes++;
new_leaf_blk = find_free_blk(ip->i_sbd);
dir_split_leaf(ip, lindex, leafblk, lbh);
/* re-read the leaf to pick up dir_split_leaf's changes */
- gfs2_leaf_in(leaf, lbh);
- *proper_len = 1 << (ip->i_di.di_depth -
- be16_to_cpu(leaf->lf_depth));
+ gfs2_leaf_in(&leaf, lbh);
+ *proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth);
log_err(_("Leaf block %llu (0x%llx) was split from length "
"%d to %d\n"), (unsigned long long)leafblk,
(unsigned long long)leafblk, len, *proper_len);
if (*proper_len < 0) {
log_err(_("Programming error: proper_len=%d, "
"di_depth = %d, lf_depth = %d.\n"),
- *proper_len, ip->i_di.di_depth,
- be16_to_cpu(leaf->lf_depth));
+ *proper_len, ip->i_di.di_depth, leaf.lf_depth);
exit(FSCK_ERROR);
}
log_err(_("New split-off leaf block was allocated at %lld "
@@ -1219,8 +1284,8 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize,
if (*proper_len < len) {
log_err(_("There are %d pointers, but leaf 0x%llx's "
"depth, %d, only allows %d\n"),
- len, (unsigned long long)leafblk,
- be16_to_cpu(leaf->lf_depth), *proper_len);
+ len, (unsigned long long)leafblk, leaf.lf_depth,
+ *proper_len);
}
brelse(lbh);
/* At this point, lindex should be at the proper end of the pointers.
@@ -1422,13 +1487,6 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl,
return error;
}
-static int pass2_repair_leaf(struct gfs2_inode *ip, uint64_t *leaf_no,
- int lindex, int ref_count, const char *msg,
- void *private)
-{
- return repair_leaf(ip, leaf_no, lindex, ref_count, msg);
-}
-
struct metawalk_fxns pass2_fxns = {
.private = NULL,
.check_leaf_depth = check_leaf_depth,
--
1.7.11.7
next prev parent reply other threads:[~2013-04-08 14:41 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-08 14:40 [Cluster-devel] [PATCH 00/42] fsck.gfs2 fixes and improvements rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 01/42] libgfs2: externalize dir_split_leaf rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 02/42] libgfs2: allow dir_split_leaf to receive a leaf buffer rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 03/42] libgfs2: let dir_split_leaf receive a "broken" lindex rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 04/42] fsck.gfs2: Move function find_free_blk to util.c rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 05/42] fsck.gfs2: Split out function to make sure lost+found exists rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 06/42] fsck.gfs2: Check for formal inode mismatch when adding to lost+found rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 07/42] fsck.gfs2: shorten some debug messages in lost+found rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 08/42] fsck.gfs2: Move basic directory entry checks to separate function rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 09/42] fsck.gfs2: Add formal inode check to basic dirent checks rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 10/42] fsck.gfs2: Add new function to check dir hash tables rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 11/42] fsck.gfs2: Special case '..' when processing bad formal inode number rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 12/42] fsck.gfs2: Move function to read directory hash table to util.c rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 13/42] fsck.gfs2: Misc cleanups rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 14/42] fsck.gfs2: Verify dirent hash values correspond to proper leaf block rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 15/42] fsck.gfs2: re-read hash table if directory height or depth changes rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 16/42] fsck.gfs2: fix leaf blocks, don't try to patch the hash table rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 17/42] fsck.gfs2: check leaf depth when validating leaf blocks rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 18/42] fsck.gfs2: small cleanups rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 19/42] fsck.gfs2: reprocess inodes when blocks are added rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 20/42] fsck.gfs2: Remove redundant leaf depth check rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 21/42] fsck.gfs2: link dinodes that only have extended attribute problems rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 22/42] fsck.gfs2: Add clarifying message to duplicate processing rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 23/42] fsck.gfs2: separate function to calculate metadata block header size rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 24/42] fsck.gfs2: Rework the "undo" functions rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 25/42] fsck.gfs2: Check for interrupt when resolving duplicates rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 26/42] fsck.gfs2: Consistent naming of struct duptree variables rpeterso
2013-04-08 14:40 ` [Cluster-devel] [PATCH 27/42] fsck.gfs2: Keep proper counts when duplicates are found rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 28/42] fsck.gfs2: print metadata block reference on data errors rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 29/42] fsck.gfs2: print block count values when fixing them rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 30/42] fsck.gfs2: Do not invalidate metablocks of dinodes with invalid mode rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 31/42] fsck.gfs2: Log when unrecoverable data block errors are encountered rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 32/42] fsck.gfs2: don't remove buffers from the list when errors are found rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 33/42] fsck.gfs2: Don't flag GFS1 non-dinode blocks as duplicates rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 34/42] fsck.gfs2: externalize check_leaf rpeterso
2013-04-08 14:41 ` rpeterso [this message]
2013-04-08 14:41 ` [Cluster-devel] [PATCH 36/42] fsck.gfs2: standardize check_metatree return codes rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 37/42] fsck.gfs2: don't invalidate files with duplicate data block refs rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 38/42] fsck.gfs2: check for duplicate first references rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 39/42] fsck.gfs2: When flagging a duplicate reference, show valid or invalid rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 40/42] fsck.gfs2: major duplicate reference reform rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 41/42] fsck.gfs2: Remove all bad eattr blocks rpeterso
2013-04-08 14:41 ` [Cluster-devel] [PATCH 42/42] fsck.gfs2: Remove unused variable 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=1365432074-17615-36-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).