Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=78c9f0a7eaa2c2... Commit: 78c9f0a7eaa2c2aecb7ecbef518fb53a2bb6a871 Parent: bf9a851204b747f850a3a95a907cd8101bc2b726 Author: Bob Peterson rpeterso@redhat.com AuthorDate: Fri Apr 19 09:25:51 2013 -0700 Committer: Bob Peterson rpeterso@redhat.com CommitterDate: Fri May 17 15:33:36 2013 -0500
fsck.gfs2: take hash table start boundaries into account
When checking the hash table in pass2, we can't just keep doubling the length for each consecutive check because the number of pointer copies (aka length) is also tied to the starting offset. If the starting offset is invalid for the length, it might treat a chunk of the hash table as bigger than it should, eventually overwriting good entries. Along the same lines, while we're trying to determine the length, it's not good enough to double the length and check if the hash table entry matches. The reason is: there can be several values overwritten with the same value, 0x00, that indicates places where pass1 found an invalid leaf block pointer. To avoid that, we need to check intermediate values as well, and stop if we find a gap.
rhbz#902920 --- gfs2/fsck/metawalk.c | 5 +++-- gfs2/fsck/pass2.c | 43 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 78047ec..1d56490 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -471,11 +471,12 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
if ((char *)dent + de.de_rec_len >= bh_end){ log_debug( _("Last entry processed for %lld->%lld " - "(0x%llx->0x%llx).\n"), + "(0x%llx->0x%llx), di_blocks=%llu.\n"), (unsigned long long)ip->i_di.di_num.no_addr, (unsigned long long)bh->b_blocknr, (unsigned long long)ip->i_di.di_num.no_addr, - (unsigned long long)bh->b_blocknr); + (unsigned long long)bh->b_blocknr, + (unsigned long long)ip->i_di.di_blocks); break; }
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c index 4b3a5c0..4b3efd9 100644 --- a/gfs2/fsck/pass2.c +++ b/gfs2/fsck/pass2.c @@ -368,9 +368,10 @@ static int wrong_leaf(struct gfs2_inode *ip, struct gfs2_inum *entry, gfs2_get_leaf_nr(ip, hash_index, &real_leaf); if (real_leaf != planned_leaf) { log_err(_("The planned leaf was split. The new leaf " - "is: %llu (0x%llx)"), + "is: %llu (0x%llx). di_blocks=%llu\n"), (unsigned long long)real_leaf, - (unsigned long long)real_leaf); + (unsigned long long)real_leaf, + (unsigned long long)ip->i_di.di_blocks); fsck_blockmap_set(ip, real_leaf, _("split leaf"), gfs2_indir_blk); } @@ -1030,6 +1031,7 @@ static int basic_check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent, log_err( _("Bad directory entry '%s' cleared.\n"), tmp_name); return 1; } else { + (*count)++; return 0; } } @@ -1148,11 +1150,13 @@ static int fix_hashtable(struct gfs2_inode *ip, uint64_t *tbl, unsigned hsize, /* Look at the first dirent and check its hash value to see if it's at the proper starting offset. */ hash_index = hash_table_index(dentry.de_hash, ip); + /* Need to use len here, not *proper_len because the leaf block may + be valid within the range, but starts too soon in the hash table. */ if (hash_index < lindex || hash_index > lindex + len) { log_err(_("This leaf block has hash index %d, which is out of " "bounds for where it appears in the hash table " "(%d - %d)\n"), - hash_index, lindex, lindex + len); + hash_index, lindex, lindex + *proper_len); error = lost_leaf(ip, tbl, leafblk, len, lindex, lbh); brelse(lbh); return error; @@ -1289,6 +1293,8 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, struct gfs2_buffer_head *lbh; int factor; uint32_t proper_start; + uint32_t next_proper_start; + int anomaly;
lindex = 0; while (lindex < hsize) { @@ -1297,10 +1303,23 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, len = 1; factor = 0; leafblk = be64_to_cpu(tbl[lindex]); + next_proper_start = lindex; + anomaly = 0; while (lindex + (len << 1) - 1 < hsize) { if (be64_to_cpu(tbl[lindex + (len << 1) - 1]) != leafblk) break; + next_proper_start = (lindex & ~((len << 1) - 1)); + if (lindex != next_proper_start) + anomaly = 1; + /* Check if there are other values written between + here and the next factor. */ + for (i = len; !anomaly && i + lindex < hsize && + i < (len << 1); i++) + if (be64_to_cpu(tbl[lindex + i]) != leafblk) + anomaly = 1; + if (anomaly) + break; len <<= 1; factor++; } @@ -1342,8 +1361,10 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, proper_start = (lindex & ~(proper_len - 1)); if (lindex != proper_start) { log_debug(_("lindex 0x%llx is not a proper starting " - "point for this leaf: 0x%llx\n"), + "point for leaf %llu (0x%llx): 0x%llx\n"), (unsigned long long)lindex, + (unsigned long long)leafblk, + (unsigned long long)leafblk, (unsigned long long)proper_start); changes = fix_hashtable(ip, tbl, hsize, leafblk, lindex, proper_start, len, @@ -1366,9 +1387,11 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, depth, and adjust the hash table accordingly. */ if (len != proper_len) { log_err(_("Length %d (0x%x) is not a proper length " - "for this leaf. Valid boundary assumed to " - "be %d (0x%x).\n"), - len, len, proper_len, proper_len); + "for leaf %llu (0x%llx). Valid boundary " + "assumed to be %d (0x%x).\n"), len, len, + (unsigned long long)leafblk, + (unsigned long long)leafblk, + proper_len, proper_len); lbh = bread(ip->i_sbd, leafblk); gfs2_leaf_in(&leaf, lbh); if (gfs2_check_meta(lbh, GFS2_METATYPE_LF) || @@ -1417,8 +1440,10 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl, proper_len = 1 << (ip->i_di.di_depth - leaf.lf_depth); if (proper_len != len) { log_debug(_("Length 0x%x is not proper for " - "this leaf: 0x%x"), - len, proper_len); + "leaf %llu (0x%llx): 0x%x"), + len, (unsigned long long)leafblk, + (unsigned long long)leafblk, + proper_len); changes = fix_hashtable(ip, tbl, hsize, leafblk, lindex, lindex, len,