174 lines
5.5 KiB
Diff
174 lines
5.5 KiB
Diff
From 100c229847e8c596eeb4eb92a71e9efc755e1558 Mon Sep 17 00:00:00 2001
|
|
From: Andrew Price <anprice@redhat.com>
|
|
Date: Fri, 18 Oct 2019 16:07:44 +0100
|
|
Subject: [PATCH] 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 d256dd2f..5792be56 100644
|
|
--- a/gfs2/fsck/metawalk.c
|
|
+++ b/gfs2/fsck/metawalk.c
|
|
@@ -1210,6 +1210,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
|
|
@@ -1229,8 +1274,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]);
|
|
|
|
@@ -1294,65 +1339,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 119efeed..b5a037a3 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
|
|
--
|
|
2.33.0
|
|
|