4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Qu Wenruo <w...@suse.com>

commit 69fc6cbbac542c349b3d350d10f6e394c253c81d upstream.

[BUG]
If we run btrfs with CONFIG_BTRFS_FS_RUN_SANITY_TESTS=y, it will
instantly cause kernel panic like:

------
...
assertion failed: 0, file: fs/btrfs/disk-io.c, line: 3853
...
Call Trace:
 btrfs_mark_buffer_dirty+0x187/0x1f0 [btrfs]
 setup_items_for_insert+0x385/0x650 [btrfs]
 __btrfs_drop_extents+0x129a/0x1870 [btrfs]
...
-----

[Cause]
Btrfs will call btrfs_check_leaf() in btrfs_mark_buffer_dirty() to check
if the leaf is valid with CONFIG_BTRFS_FS_RUN_SANITY_TESTS=y.

However quite some btrfs_mark_buffer_dirty() callers(*) don't really
initialize its item data but only initialize its item pointers, leaving
item data uninitialized.

This makes tree-checker catch uninitialized data as error, causing
such panic.

*: These callers include but not limited to
setup_items_for_insert()
btrfs_split_item()
btrfs_expand_item()

[Fix]
Add a new parameter @check_item_data to btrfs_check_leaf().
With @check_item_data set to false, item data check will be skipped and
fallback to old btrfs_check_leaf() behavior.

So we can still get early warning if we screw up item pointers, and
avoid false panic.

Cc: Filipe Manana <fdman...@gmail.com>
Reported-by: Lakshmipathi.G <lakshmipath...@gmail.com>
Signed-off-by: Qu Wenruo <w...@suse.com>
Reviewed-by: Liu Bo <bo.li....@oracle.com>
Reviewed-by: David Sterba <dste...@suse.com>
Signed-off-by: David Sterba <dste...@suse.com>
[bwh: Backported to 4.4: adjust context]
Signed-off-by: Ben Hutchings <ben.hutchi...@codethink.co.uk>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 fs/btrfs/disk-io.c      |   10 ++++++++--
 fs/btrfs/tree-checker.c |   27 ++++++++++++++++++++++-----
 fs/btrfs/tree-checker.h |   14 +++++++++++++-
 3 files changed, 43 insertions(+), 8 deletions(-)

--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -589,7 +589,7 @@ static int btree_readpage_end_io_hook(st
         * that we don't try and read the other copies of this block, just
         * return -EIO.
         */
-       if (found_level == 0 && btrfs_check_leaf(root, eb)) {
+       if (found_level == 0 && btrfs_check_leaf_full(root, eb)) {
                set_bit(EXTENT_BUFFER_CORRUPT, &eb->bflags);
                ret = -EIO;
        }
@@ -3896,7 +3896,13 @@ void btrfs_mark_buffer_dirty(struct exte
                                     buf->len,
                                     root->fs_info->dirty_metadata_batch);
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
-       if (btrfs_header_level(buf) == 0 && btrfs_check_leaf(root, buf)) {
+       /*
+        * Since btrfs_mark_buffer_dirty() can be called with item pointer set
+        * but item data not updated.
+        * So here we should only check item pointers, not item data.
+        */
+       if (btrfs_header_level(buf) == 0 &&
+           btrfs_check_leaf_relaxed(root, buf)) {
                btrfs_print_leaf(root, buf);
                ASSERT(0);
        }
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -195,7 +195,8 @@ static int check_leaf_item(struct btrfs_
        return ret;
 }
 
-int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
+static int check_leaf(struct btrfs_root *root, struct extent_buffer *leaf,
+                     bool check_item_data)
 {
        struct btrfs_fs_info *fs_info = root->fs_info;
        /* No valid key type is 0, so all key should be larger than this key */
@@ -299,10 +300,15 @@ int btrfs_check_leaf(struct btrfs_root *
                        return -EUCLEAN;
                }
 
-               /* Check if the item size and content meet other criteria */
-               ret = check_leaf_item(root, leaf, &key, slot);
-               if (ret < 0)
-                       return ret;
+               if (check_item_data) {
+                       /*
+                        * Check if the item size and content meet other
+                        * criteria
+                        */
+                       ret = check_leaf_item(root, leaf, &key, slot);
+                       if (ret < 0)
+                               return ret;
+               }
 
                prev_key.objectid = key.objectid;
                prev_key.type = key.type;
@@ -312,6 +318,17 @@ int btrfs_check_leaf(struct btrfs_root *
        return 0;
 }
 
+int btrfs_check_leaf_full(struct btrfs_root *root, struct extent_buffer *leaf)
+{
+       return check_leaf(root, leaf, true);
+}
+
+int btrfs_check_leaf_relaxed(struct btrfs_root *root,
+                            struct extent_buffer *leaf)
+{
+       return check_leaf(root, leaf, false);
+}
+
 int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
 {
        unsigned long nr = btrfs_header_nritems(node);
--- a/fs/btrfs/tree-checker.h
+++ b/fs/btrfs/tree-checker.h
@@ -20,7 +20,19 @@
 #include "ctree.h"
 #include "extent_io.h"
 
-int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf);
+/*
+ * Comprehensive leaf checker.
+ * Will check not only the item pointers, but also every possible member
+ * in item data.
+ */
+int btrfs_check_leaf_full(struct btrfs_root *root, struct extent_buffer *leaf);
+
+/*
+ * Less strict leaf checker.
+ * Will only check item pointers, not reading item data.
+ */
+int btrfs_check_leaf_relaxed(struct btrfs_root *root,
+                            struct extent_buffer *leaf);
 int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node);
 
 #endif


Reply via email to