Skip to contentSkip to conversation

btrfs: tree-checker: Check level for leaves and nodes

PATCH In review Qu Wenruo 5 participants 5 messages Sep 27, 2018 – Oct 2, 2018 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

PATCH 1/1 btrfs: tree-checker: Check level for leaves and nodes

Although we have tree level check at tree read runtime, it's completely based on its parent level.
We still need to do accurate level check to avoid invalid tree blocks sneak into kernel space.

The check itself is simple, for leaf its level should always be 0. For nodes its level should be in range [1, BTRFS_MAX_LEVEL - 1].

---
 fs/btrfs/tree-checker.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
fs/btrfs/tree-checker.c +14 -0
index db835635372f..cab0b1f1f741 100644
@@ -487,6 +487,13 @@ static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer *leaf,
 	u32 nritems = btrfs_header_nritems(leaf);
 	int slot;
 
+	if (btrfs_header_level(leaf) != 0) {
+		generic_err(fs_info, leaf, 0,
+			"invalid level for leaf, have %d expect 0",
+			btrfs_header_level(leaf));
+		return -EUCLEAN;
+	}
+
 	/*
 	 * Extent buffers from a relocation tree have a owner field that
 	 * corresponds to the subvolume tree they are based on. So just from an
@@ -645,9 +652,16 @@ int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer *node)
 	unsigned long nr = btrfs_header_nritems(node);
 	struct btrfs_key key, next_key;
 	int slot;
+	int level = btrfs_header_level(node);
 	u64 bytenr;
 	int ret = 0;
 
+	if (level <= 0 || level >= BTRFS_MAX_LEVEL) {
+		generic_err(fs_info, node, 0,
+			"invalid level for node, have %d expect [1, %d]",
+			level, BTRFS_MAX_LEVEL - 1);
+		return -EUCLEAN;
+	}
 	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(fs_info)) {
 		btrfs_crit(fs_info,
 "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",
Signature · 1 line
2.19.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 12 lines

Sent: Friday, September 28, 2018 at 7:59 AM
From: "Qu Wenruo" <[email protected]>
To: [email protected]
Subject: [PATCH] btrfs: tree-checker: Check level for leaves and nodes

Although we have tree level check at tree read runtime, it's completely based on its parent level.
We still need to do accurate level check to avoid invalid tree blocks sneak into kernel space.

Quoted 4 lines

The check itself is simple, for leaf its level should always be 0. For nodes its level should be in range [1, BTRFS_MAX_LEVEL - 1].

Quoted 43 lines
---
 fs/btrfs/tree-checker.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
fs/btrfs/tree-checker.c +14 -0
index db835635372f..cab0b1f1f741 100644
@@ -487,6 +487,13 @@ static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer *leaf,
 	u32 nritems = btrfs_header_nritems(leaf);
 	int slot;
 
+	if (btrfs_header_level(leaf) != 0) {
+		generic_err(fs_info, leaf, 0,
+			"invalid level for leaf, have %d expect 0",
+			btrfs_header_level(leaf));
+		return -EUCLEAN;
+	}
+
 	/*
 	 * Extent buffers from a relocation tree have a owner field that
 	 * corresponds to the subvolume tree they are based on. So just from an
@@ -645,9 +652,16 @@ int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer *node)
 	unsigned long nr = btrfs_header_nritems(node);
 	struct btrfs_key key, next_key;
 	int slot;
+	int level = btrfs_header_level(node);
 	u64 bytenr;
 	int ret = 0;
 
+	if (level <= 0 || level >= BTRFS_MAX_LEVEL) {
+		generic_err(fs_info, node, 0,
+			"invalid level for node, have %d expect [1, %d]",
+			level, BTRFS_MAX_LEVEL - 1);
+		return -EUCLEAN;
+	}
 	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(fs_info)) {
 		btrfs_crit(fs_info,
 "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",
Signature · 1 line
2.19.0
#2 lore raw
In reply to Qu Wenruo
Quoted 18 lines · Qu Wenruo

Although we have tree level check at tree read runtime, it's completely based on its parent level.
We still need to do accurate level check to avoid invalid tree blocks sneak into kernel space.

The check itself is simple, for leaf its level should always be 0. For nodes its level should be in range [1, BTRFS_MAX_LEVEL - 1].

---
  fs/btrfs/tree-checker.c | 14 ++++++++++++++
  1 file changed, 14 insertions(+)
fs/btrfs/tree-checker.c +0 -0
index db835635372f..cab0b1f1f741 100644

fs/btrfs/tree-checker.c static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer *leaf, · line 496

Quoted 10 lines
fs/btrfs/tree-checker.c +7 -0
@@ -487,6 +487,13 @@ static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer *leaf,
  	u32 nritems = btrfs_header_nritems(leaf);
  	int slot;
 
+	if (btrfs_header_level(leaf) != 0) {
+		generic_err(fs_info, leaf, 0,
+			"invalid level for leaf, have %d expect 0",
+			btrfs_header_level(leaf));
+		return -EUCLEAN;
+	}
+

BTW, output more info(bytenr, root) of the corrupted node/leaf may be better?

Quoted 21 lines
fs/btrfs/tree-checker.c +7 -0
  	/*
  	 * Extent buffers from a relocation tree have a owner field that
  	 * corresponds to the subvolume tree they are based on. So just from an
@@ -645,9 +652,16 @@ int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer *node)
  	unsigned long nr = btrfs_header_nritems(node);
  	struct btrfs_key key, next_key;
  	int slot;
+	int level = btrfs_header_level(node);
  	u64 bytenr;
  	int ret = 0;
 
+	if (level <= 0 || level >= BTRFS_MAX_LEVEL) {
+		generic_err(fs_info, node, 0,
+			"invalid level for node, have %d expect [1, %d]",
+			level, BTRFS_MAX_LEVEL - 1);
+		return -EUCLEAN;
+	}
  	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(fs_info)) {
  		btrfs_crit(fs_info,
  "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",
#3 lore raw
Quoted 25 lines · Su Yue
Quoted 22 lines · Qu Wenruo

Although we have tree level check at tree read runtime, it's completely based on its parent level.
We still need to do accurate level check to avoid invalid tree blocks sneak into kernel space.

The check itself is simple, for leaf its level should always be 0. For nodes its level should be in range [1, BTRFS_MAX_LEVEL - 1].

---
  fs/btrfs/tree-checker.c | 14 ++++++++++++++

  1 file changed, 14 insertions(+)
fs/btrfs/tree-checker.c +0 -0
index db835635372f..cab0b1f1f741 100644
@@ -487,6 +487,13 @@ static int check_leaf(struct btrfs_fs_info
 *fs_info, struct extent_buffer *leaf,
       u32 nritems = btrfs_header_nritems(leaf);
       int slot;
   +    if (btrfs_header_level(leaf) != 0) {
Quoted 8 lines

fs/btrfs/tree-checker.c static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer *leaf, · line 496

Quoted 6 lines
fs/btrfs/tree-checker.c +6 -0
+        generic_err(fs_info, leaf, 0,
+            "invalid level for leaf, have %d expect 0",
+            btrfs_header_level(leaf));
+        return -EUCLEAN;
+    }
+

BTW, output more info(bytenr, root) of the corrupted node/leaf may be better?

That's what will be outputted by generic_err() function.

Thanks,
Qu

Quoted 27 lines
Quoted 24 lines
fs/btrfs/tree-checker.c +0 -0
       /*
        * Extent buffers from a relocation tree have a owner field that

       * corresponds to the subvolume tree they are based on. So just from an

fs/btrfs/tree-checker.c +6 -0
@@ -645,9 +652,16 @@ int btrfs_check_node(struct btrfs_fs_info
 *fs_info, struct extent_buffer *node)
       unsigned long nr = btrfs_header_nritems(node);
       struct btrfs_key key, next_key;
       int slot;
+    int level = btrfs_header_level(node);
       u64 bytenr;
       int ret = 0;
   +    if (level <= 0 || level >= BTRFS_MAX_LEVEL) {
+        generic_err(fs_info, node, 0,
+            "invalid level for node, have %d expect [1, %d]",
+            level, BTRFS_MAX_LEVEL - 1);
+        return -EUCLEAN;
+    }
       if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(fs_info)) {
           btrfs_crit(fs_info,

  "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",

Reviewed-by on v1 1/1
#4 lore raw
In reply to Qu Wenruo
Quoted 9 lines · Qu Wenruo

Although we have tree level check at tree read runtime, it's completely based on its parent level.
We still need to do accurate level check to avoid invalid tree blocks sneak into kernel space.

The check itself is simple, for leaf its level should always be 0. For nodes its level should be in range [1, BTRFS_MAX_LEVEL - 1].

Added to misc-next.