From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: Leonard Lausen <leonard@lausen.nl>, David Sterba <dsterba@suse.com>
Subject: [PATCH 2/2] btrfs: Do mandatory tree block check before submitting bio
Date: Thu, 4 Apr 2019 11:47:08 +0800 [thread overview]
Message-ID: <20190404034708.3399-3-wqu@suse.com> (raw)
In-Reply-To: <20190404034708.3399-1-wqu@suse.com>
There are at least 2 reports about a memory bit flip sneaking into
on-disk data.
Currently we only have a relaxed check triggered at
btrfs_mark_buffer_dirty() time, as it's not mandatory and only for
CONFIG_BTRFS_FS_CHECK_INTEGRITY enabled build, it doesn't help users to
detect such problem.
This patch will address the hole by triggering comprehensive check on
tree blocks before writing it back to disk.
The design points are:
- Timing of the check: Tree block write hook
This timing is chosen to reduce the overhead.
The comprehensive check should be as expensive as a checksum
calculation.
Doing full check at btrfs_mark_buffer_dirty() is too expensive for end
user.
- Loose empty leaf check
Originally for an empty leaf, tree-checker will report error if it's
not a tree root.
The problem for such check at write time is:
* False alert for tree root created in current transaction
In that case, the commit root still needs to be written to disk.
And since current root can differ from commit root, then it will
cause false alert.
This happens for log tree.
* False alert for relocated tree block
Relocated tree block can be written to disk due to memory pressure,
in that case an empty csum tree root can be written to disk and
cause false alert, since csum root node hasn't been updated.
Previous patch of removing comprehensive empty leaf owner check has
paved the way for this patch.
The example error output will be something like:
BTRFS critical (device dm-3): corrupt leaf: root=2 block=1350630375424 slot=68, bad key order, prev (10510212874240 169 0) current (1714119868416 169 0)
BTRFS error (device dm-3): block=1350630375424 write time tree block corruption detected
BTRFS: error (device dm-3) in btrfs_commit_transaction:2220: errno=-5 IO failure (Error while writing out transaction)
BTRFS info (device dm-3): forced readonly
BTRFS warning (device dm-3): Skipping commit of aborted transaction.
BTRFS: error (device dm-3) in cleanup_transaction:1839: errno=-5 IO failure
BTRFS info (device dm-3): delayed_refs has NO entry
Reported-by: Leonard Lausen <leonard@lausen.nl>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/disk-io.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0b2b75a7efbd..c2c0640aea55 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -514,6 +514,7 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
u8 result[BTRFS_CSUM_SIZE];
u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
struct extent_buffer *eb;
+ int err;
eb = (struct extent_buffer *)page->private;
if (page != eb->pages[0])
@@ -535,7 +536,19 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
if (csum_tree_block(eb, result))
return -EINVAL;
+ if (btrfs_header_level(eb))
+ err = btrfs_check_node(fs_info, eb);
+ else
+ err = btrfs_check_leaf_full(fs_info, eb);
+
+ if (err < 0) {
+ btrfs_err(fs_info,
+ "block=%llu write time tree block corruption detected",
+ eb->start);
+ return err;
+ }
write_extent_buffer(eb, result, 0, csum_size);
+
return 0;
}
--
2.21.0
next prev parent reply other threads:[~2019-04-04 3:47 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-04 3:47 [PATCH 0/2] Fixup and optimization for write time tree checker Qu Wenruo
2019-04-04 3:47 ` [PATCH 1/2] btrfs: tree-checker: Remove comprehensive root owner check Qu Wenruo
2019-04-04 6:23 ` Nikolay Borisov
2019-04-04 6:33 ` Qu Wenruo
2019-04-04 15:24 ` David Sterba
2019-04-04 3:47 ` Qu Wenruo [this message]
2019-04-12 15:36 ` [PATCH 2/2] btrfs: Do mandatory tree block check before submitting bio David Sterba
2019-04-05 15:49 ` [PATCH 0/2] Fixup and optimization for write time tree checker David Sterba
2019-04-06 0:22 ` Qu Wenruo
2019-04-06 1:57 ` Qu Wenruo
2019-04-08 22:18 ` David Sterba
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=20190404034708.3399-3-wqu@suse.com \
--to=wqu@suse.com \
--cc=dsterba@suse.com \
--cc=leonard@lausen.nl \
--cc=linux-btrfs@vger.kernel.org \
/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).