Hi, Qu,
On Wed, Jan 16, 2019 at 07:53:08PM +0800, Qu Wenruo wrote:
> There are at least 2 reports about 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, only for
> CONFIG_BTRFS_FS_CHECK_INTEGRITY enabled build.
>
> This patch will address the hole by triggering comprehensive check on
> tree blocks before writing it back to disk.
>
> The timing is set to csum_tree_block() where @verify == 0.
> At that timing, we're generation csum for tree blocks before submitting
> the metadata bio, so we could avoid all the unnecessary calls at
> btrfs_mark_buffer_dirty(), but still catch enough error.
I agree wholeheartedly with the idea of this change. Just one
question:
How does this get reported to the user/administrator? As I
understand it, a detectably corrupt metadata page will generate an I/O
error from the filesystem before it's written to disk? How will this
show up in kernel logs? Is it distinguishable in any way from a
similar error that was generated on reading such a corrupt metadata
node from the disk?
Basically, I want to be able to distinguish this case (error
detected when writing) from the existing case (error detected when
reading) when someone shows up on IRC with a "broken filesystem".
Hugo.
> Reported-by: Leonard Lausen <leonard@xxxxxxxxx>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> fs/btrfs/disk-io.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 8da2f380d3c0..45bf6be9e751 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -313,6 +313,16 @@ static int csum_tree_block(struct btrfs_fs_info *fs_info,
> return -EUCLEAN;
> }
> } else {
> + /*
> + * Here we're calculating csum before writing it to disk,
> + * do comprenhensive check here to catch memory corruption
> + */
> + if (btrfs_header_level(buf))
> + err = btrfs_check_node(fs_info, buf);
> + else
> + err = btrfs_check_leaf_full(fs_info, buf);
> + if (err < 0)
> + return err;
> write_extent_buffer(buf, result, 0, csum_size);
> }
>
--
Hugo Mills | ... one ping(1) to rule them all, and in the
hugo@... carfax.org.uk | darkness bind(2) them.
http://carfax.org.uk/ |
PGP: E2AB1DE4 | Illiad
Attachment:
signature.asc
Description: Digital signature
