This is an automated email from the git hooks/post-receive script.
andyp pushed a commit to branch RHEL7 in repository gfs2-utils.
commit ab1c367459b5fd71eb80ad40645f25c21b95e70d Author: Andrew Price anprice@redhat.com AuthorDate: Fri Oct 18 16:07:44 2019 +0100
fsck.gfs2: Fix segfault in build_and_check_metalist
In unlikely circumstances, indirect pointer corruption in a 'system' inode's metadata tree can lead to the inode block state being marked as 'free' in pass1, which causes build_and_check_metalist() to be called in pass 2. The pass has a NULL ->check_metalist function pointer and so a segfault occurs when build_and_check_metalist attempts to call it.
Fix the segfault by calling ->check_metalist() only when it's not NULL. This required some refactoring to make the extra level of if-nesting easier to implement and read.
Resolves: rhbz#1487726
Signed-off-by: Andrew Price anprice@redhat.com --- gfs2/fsck/metawalk.c | 107 +++++++++++++++++++++++---------------------------- gfs2/fsck/metawalk.h | 1 + 2 files changed, 50 insertions(+), 58 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 4d6a2d2..dbe9f1f 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1207,6 +1207,51 @@ 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) +{ + int was_duplicate = 0; + int is_valid = 1; + int error; + + if (pass->check_metalist == NULL) + return 0; + + error = pass->check_metalist(ip, block, bhp, height, &is_valid, + &was_duplicate, pass->private); + if (error == meta_error) { + stack; + log_info("\n"); + log_info(_("Serious metadata error on block %"PRIu64" (0x%"PRIx64").\n"), + block, block); + return error; + } + if (error == meta_skip_further) { + log_info("\n"); + log_info(_("Unrecoverable metadata error on block %"PRIu64" (0x%"PRIx64")\n"), + block, block); + log_info(_("Further metadata will be skipped.\n")); + return error; + } + if (!is_valid) { + log_debug("Skipping rejected block %"PRIu64" (0x%"PRIx64")\n", block, block); + if (pass->invalid_meta_is_fatal) + return meta_error; + return meta_skip_one; + } + if (was_duplicate) { + log_debug("Skipping duplicate %"PRIu64" (0x%"PRIx64")\n", block, block); + return meta_skip_one; + } + if (!valid_block_ip(ip, block)) { + log_debug("Skipping invalid block %"PRIu64" (0x%"PRIx64")\n", block, block); + if (pass->invalid_meta_is_fatal) + return meta_error; + return meta_skip_one; + } + return error; +} + /** * build_and_check_metalist - check a bunch of indirect blocks * This includes hash table blocks for directories @@ -1226,8 +1271,8 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, osi_list_t *prev_list, *cur_list, *tmp; int h, head_size, iblk_type; uint64_t *ptr, block, *undoptr; - int error, was_duplicate, is_valid; int maxptrs; + int error;
osi_list_add(&metabh->b_altlist, &mlp[0]);
@@ -1291,65 +1336,11 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, continue;
block = be64_to_cpu(*ptr); - was_duplicate = 0; - error = pass->check_metalist(ip, block, &nbh, - h, &is_valid, - &was_duplicate, - pass->private); - /* check_metalist should hold any buffers - it gets with "bread". */ - if (error == meta_error) { - stack; - log_info(_("\nSerious metadata " - "error on block %llu " - "(0x%llx).\n"), - (unsigned long long)block, - (unsigned long long)block); + error = do_check_metalist(ip, block, h, &nbh, pass); + if (error == meta_error || error == meta_skip_further) goto error_undo; - } - if (error == meta_skip_further) { - log_info(_("\nUnrecoverable metadata " - "error on block %llu " - "(0x%llx). Further metadata" - " will be skipped.\n"), - (unsigned long long)block, - (unsigned long long)block); - goto error_undo; - } - if (!is_valid) { - log_debug( _("Skipping rejected block " - "%llu (0x%llx)\n"), - (unsigned long long)block, - (unsigned long long)block); - if (pass->invalid_meta_is_fatal) { - error = meta_error; - goto error_undo; - } - continue; - } - /* Note that there's a special case in which - we need to process the metadata block, even - if it was a duplicate. That's for cases - where we deleted the last reference as - metadata. */ - if (was_duplicate) { - log_debug( _("Skipping duplicate %llu " - "(0x%llx)\n"), - (unsigned long long)block, - (unsigned long long)block); + if (error == meta_skip_one) continue; - } - if (!valid_block_ip(ip, block)) { - log_debug( _("Skipping invalid block " - "%lld (0x%llx)\n"), - (unsigned long long)block, - (unsigned long long)block); - if (pass->invalid_meta_is_fatal) { - error = meta_error; - goto error_undo; - } - continue; - } if (!nbh) nbh = bread(ip->i_sbd, block); osi_list_add_prev(&nbh->b_altlist, cur_list); diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h index 119efee..b5a037a 100644 --- a/gfs2/fsck/metawalk.h +++ b/gfs2/fsck/metawalk.h @@ -39,6 +39,7 @@ enum meta_check_rc { meta_error = -1, meta_is_good = 0, meta_skip_further = 1, + meta_skip_one = 2, };
/* metawalk_fxns: function pointers to check various parts of the fs