[v2] btrfs-progs: Enhance read_tree_block to avoid memory corruption

Commit Message

Qu Wenruo May 22, 2015, 1:01 a.m. UTC

Add the following tree block check to avoid memory corruption on hostile
image:
1) Check level.
Level >= BTRFS_MAX_LEVEL won't be read out.

2) Nritems.
For nr_items > max_nritems, the tree_block won't be read out.
Max nritems is calculated in a easy method.
For node, it's straightforward, just (nodesize - header size) /
(btrfs_key_ptr)
For leaf, (nodesize - header size) / (btrfs_item), as btrfs support zero
item size

This fixes 3 kernel bugs: BZ#97171, BZ#97191, BZ#97271.

Reported-by: Lukas Lueg <[e-mail]>
Signed-off-by: Qu Wenruo <[e-mail]>
---
v2:
  Consider nr_item == 0 is valid, as empty tree like csum tree is
  allowed to exist.
---
 disk-io.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

David Sterba May 25, 2015, 1:43 p.m. UTC | #1

On Fri, May 22, 2015 at 09:01:23AM +0800, Qu Wenruo wrote:
> Add the following tree block check to avoid memory corruption on hostile
> image:
> 1) Check level.
> Level >= BTRFS_MAX_LEVEL won't be read out.
> 
> 2) Nritems.
> For nr_items > max_nritems, the tree_block won't be read out.
> Max nritems is calculated in a easy method.
> For node, it's straightforward, just (nodesize - header size) /
> (btrfs_key_ptr)
> For leaf, (nodesize - header size) / (btrfs_item), as btrfs support zero
> item size
> 
> This fixes 3 kernel bugs: BZ#97171, BZ#97191, BZ#97271.
> 
> Reported-by: Lukas Lueg <[e-mail]>
> Signed-off-by: Qu Wenruo <[e-mail]>

Applied, thanks.

I've added image for the first bug, the other two do not pass inside our
testing setup (check/repair/check) and fails to start, but the image
restoration works at least. This would need some enhancements.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/disk-io.c b/disk-io.c
index aa176ae..9de055a 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -37,8 +37,22 @@ 
 /* specified errno for check_tree_block */
 #define BTRFS_BAD_BYTENR		(-1)
 #define BTRFS_BAD_FSID			(-2)
+#define BTRFS_BAD_LEVEL			(-3)
+#define BTRFS_BAD_NRITEMS		(-4)
 
 #define IS_ALIGNED(x, a)                (((x) & ((typeof(x))(a) - 1)) == 0)
+
+/* Calculate max possible nritems for a leaf/node */
+static u32 max_nritems(u8 level, u32 nodesize)
+{
+
+	if (level == 0)
+		return ((nodesize - sizeof(struct btrfs_header)) /
+			sizeof(struct btrfs_item));
+	return ((nodesize - sizeof(struct btrfs_header)) /
+		sizeof(struct btrfs_key_ptr));
+}
+
 static int check_tree_block(struct btrfs_root *root, struct extent_buffer *buf)
 {
 
@@ -47,6 +61,11 @@  static int check_tree_block(struct btrfs_root *root, struct extent_buffer *buf)
 
 	if (buf->start != btrfs_header_bytenr(buf))
 		return BTRFS_BAD_BYTENR;
+	if (btrfs_header_level(buf) >= BTRFS_MAX_LEVEL)
+		return BTRFS_BAD_LEVEL;
+	if (btrfs_header_nritems(buf) > max_nritems(btrfs_header_level(buf),
+						    root->nodesize))
+		return BTRFS_BAD_NRITEMS;
 
 	fs_devices = root->fs_info->fs_devices;
 	while (fs_devices) {
@@ -83,6 +102,14 @@  static void print_tree_block_error(struct btrfs_root *root,
 		fprintf(stderr, "bytenr mismatch, want=%llu, have=%llu\n",
 			eb->start, btrfs_header_bytenr(eb));
 		break;
+	case BTRFS_BAD_LEVEL:
+		fprintf(stderr, "bad level, %u > %u\n",
+			btrfs_header_level(eb), BTRFS_MAX_LEVEL);
+		break;
+	case BTRFS_BAD_NRITEMS:
+		fprintf(stderr, "invalid nr_items: %u\n",
+			btrfs_header_nritems(eb));
+		break;
 	}
 }