diff mbox

[v2,4/4] btrfs: Do super block verification before writing it to disk

Message ID 20180424044809.29838-5-wqu@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Qu Wenruo April 24, 2018, 4:48 a.m. UTC
There are already 2 reports about strangely corrupted super blocks,
where csum still matches but extra garbage gets slipped into super block.

The corruption would looks like:
------
superblock: bytenr=65536, device=/dev/sdc1
---------------------------------------------------------
csum_type               41700 (INVALID)
csum                    0x3b252d3a [match]
bytenr                  65536
flags                   0x1
                        ( WRITTEN )
magic                   _BHRfS_M [match]
...
incompat_flags          0x5b22400000000169
                        ( MIXED_BACKREF |
                          COMPRESS_LZO |
                          BIG_METADATA |
                          EXTENDED_IREF |
                          SKINNY_METADATA |
                          unknown flag: 0x5b22400000000000 )
...
------
Or
------
superblock: bytenr=65536, device=/dev/mapper/x
---------------------------------------------------------
csum_type              35355 (INVALID)
csum_size              32
csum                   0xf0dbeddd [match]
bytenr                 65536
flags                  0x1
                       ( WRITTEN )
magic                  _BHRfS_M [match]
...
incompat_flags         0x176d200000000169
                       ( MIXED_BACKREF |
                         COMPRESS_LZO |
                         BIG_METADATA |
                         EXTENDED_IREF |
                         SKINNY_METADATA |
                         unknown flag: 0x176d200000000000 )
------

Obviously, csum_type and incompat_flags get some garbage, but its csum
still matches, which means kernel calculates the csum based on corrupted
super block memory.
And after manually fixing these values, the filesystem is completely
healthy without any problem exposed by btrfs check.

Although the cause is still unknown, at least detect it and prevent further
corruption.

Reported-by: Ken Swenson <flat@imo.uto.moe>
Reported-by: Ben Parsons <9parsonsb@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

Comments

David Sterba April 24, 2018, 1:14 p.m. UTC | #1
On Tue, Apr 24, 2018 at 12:48:09PM +0800, Qu Wenruo wrote:
> -static int btrfs_validate_super(struct btrfs_fs_info *fs_info)
> +/*
> + * Check the validation of btrfs super block.
> + *
> + * @sb:			super block to check
> + * @super_mirror:	the super block number to check its bytenr.
> + * 			0 means the primary (1st) sb, 1 and 2 means 2nd and
> + * 			3rd backup sb, while -1 means to skip bytenr check.

Please format the values like:

/*
 * Check the validity of btrfs super block
 *
 * @sb:                        super block to check
 * @super_mirror:      the super block number to check its bytenr:
 *                     0 means the primary (1st) sb,
 *                     1 and 2 means 2nd and 3rd backup copy
 *                     -1 means to skip bytenr check
 */

> + */
> +static int btrfs_validate_super(struct btrfs_fs_info *fs_info,
> +				struct btrfs_super_block *sb,
> +				int super_mirror)
>  {
> -	struct btrfs_super_block *sb = fs_info->super_copy;
>  	u64 nodesize = btrfs_super_nodesize(sb);
>  	u64 sectorsize = btrfs_super_sectorsize(sb);
>  	int ret = 0;
> @@ -4088,9 +4109,10 @@ static int btrfs_validate_super(struct btrfs_fs_info *fs_info)
>  		ret = -EINVAL;
>  	}
>  
> -	if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
> -		btrfs_err(fs_info, "super offset mismatch %llu != %u",
> -			  btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
> +	if (super_mirror >= 0 && btrfs_super_bytenr(sb) !=
> +	    btrfs_sb_offset(super_mirror)) {

The preferred condition split is after the operators, not in the middle
of the expression. Like this:

	if (super_mirror >= 0 &&
	    btrfs_super_bytenr(sb) != btrfs_sb_offset(super_mirror)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9282a6ac91db..0f5771244641 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -55,7 +55,9 @@ 
 static const struct extent_io_ops btree_extent_io_ops;
 static void end_workqueue_fn(struct btrfs_work *work);
 static void free_fs_root(struct btrfs_root *root);
-static int btrfs_validate_super(struct btrfs_fs_info *fs_info);
+static int btrfs_validate_super(struct btrfs_fs_info *fs_info,
+				   struct btrfs_super_block *sb,
+				   int super_mirror);
 static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
 static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 				      struct btrfs_fs_info *fs_info);
@@ -2668,7 +2670,7 @@  int open_ctree(struct super_block *sb,
 
 	memcpy(fs_info->fsid, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
 
-	ret = btrfs_validate_super(fs_info);
+	ret = btrfs_validate_super(fs_info, fs_info->super_copy, 0);
 	if (ret) {
 		btrfs_err(fs_info, "superblock contains fatal errors");
 		err = -EINVAL;
@@ -3563,6 +3565,16 @@  int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
 	sb = fs_info->super_for_commit;
 	dev_item = &sb->dev_item;
 
+	/*
+	 * super_bytenr will be updated in write_dev_supers(), even if it is
+	 * corrupted in current copy, it won't reach disk. So skip bytenr check.
+	 */
+	if (btrfs_validate_super(fs_info, sb, -1)) {
+		btrfs_err(fs_info,
+		"superblock corruption detected before transaction commit");
+		return -EUCLEAN;
+	}
+
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	head = &fs_info->fs_devices->devices;
 	max_errors = btrfs_super_num_devices(fs_info->super_copy) - 1;
@@ -3974,9 +3986,18 @@  int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
 					      level, first_key);
 }
 
-static int btrfs_validate_super(struct btrfs_fs_info *fs_info)
+/*
+ * Check the validation of btrfs super block.
+ *
+ * @sb:			super block to check
+ * @super_mirror:	the super block number to check its bytenr.
+ * 			0 means the primary (1st) sb, 1 and 2 means 2nd and
+ * 			3rd backup sb, while -1 means to skip bytenr check.
+ */
+static int btrfs_validate_super(struct btrfs_fs_info *fs_info,
+				struct btrfs_super_block *sb,
+				int super_mirror)
 {
-	struct btrfs_super_block *sb = fs_info->super_copy;
 	u64 nodesize = btrfs_super_nodesize(sb);
 	u64 sectorsize = btrfs_super_sectorsize(sb);
 	int ret = 0;
@@ -4088,9 +4109,10 @@  static int btrfs_validate_super(struct btrfs_fs_info *fs_info)
 		ret = -EINVAL;
 	}
 
-	if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
-		btrfs_err(fs_info, "super offset mismatch %llu != %u",
-			  btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
+	if (super_mirror >= 0 && btrfs_super_bytenr(sb) !=
+	    btrfs_sb_offset(super_mirror)) {
+		btrfs_err(fs_info, "super offset mismatch %llu != %llu",
+			btrfs_super_bytenr(sb), btrfs_sb_offset(super_mirror));
 		ret = -EINVAL;
 	}