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

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

[Qu Wenruo]

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

2) Nritems.
For nritems == 0 or 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), assume btrfs support
item size == 0;

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

Reported-by: Lukas Lueg <lukas.lueg@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 disk-io.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/disk-io.c b/disk-io.c
index bf796c6..356150a 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,12 @@ 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) == 0 ||
+	    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) {
@@ -82,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;
 	}
 }
 
-- 
2.4.0

Re: [PATCH] btrfs-progs: Enhance read_tree_block to avoid memory corruption.

[David Sterba]

On Thu, May 14, 2015 at 11:06:24AM +0800, Qu Wenruo wrote:
> Add the following tree_block check to avoid memory corruption or hostile
> image:
> 1) Check level.
> Level >= BTRFS_MAX_LEVEL won't be read out.
> 
> 2) Nritems.
> For nritems == 0 or 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), assume btrfs support
> item size == 0;
> 
> This fixes 3 kernel bugs: BZ#97171, BZ#97191, BZ#97271.
> 
> Reported-by: Lukas Lueg <lukas.lueg@gmail.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

The test 001-bad-file-extent-bytenr fails with this patch (and passes
otherwise). Can you please have a look?

Re: [PATCH] btrfs-progs: Enhance read_tree_block to avoid memory corruption.

[David Sterba]

On Thu, May 21, 2015 at 04:52:49PM +0200, David Sterba wrote:
> The test 001-bad-file-extent-bytenr fails with this patch (and passes
> otherwise). Can you please have a look?

First check expectedly finds problems and fails, then repair is supposed
to fix it, but the final check still finds problems because of the
enhancements you've added:

# output from the final check:
checking extents
invalid nr_items: 0
Errors found in extent allocation tree or chunk allocation
checking free space cache
checking fs roots
root 5 inode 257 errors 1000, some csum missing
Checking filesystem on .../001-bad-file-extent-bytenr/default_case.img.restored
UUID: 17442351-31aa-45fa-9503-90fd48874c3e
cache and super generation don't match, space cache will be invalidated
found 1081346 bytes used err is 1
total csum bytes: 1024
total tree bytes: 24576
total fs tree bytes: 4096
total extent tree bytes: 4096
btree space waste bytes: 16507
file data blocks allocated: 1310720
 referenced 1310720
btrfs-progs v4.0.1-2-gb7cf7c5c181d
failed: .../btrfs check .../001-bad-file-extent-bytenr/default_case.img.restored

It's "invalid nr_items: 0", while in the unpatched test there's

Device extent[1, 29360128, 8388608] didn't find the relative chunk.
Device extent[1, 1111490560, 1073741824] didn't find the relative chunk.
Dev extent's total-byte(2185232384) is not equal to byte-used(1103101952) in dev[1, 216, 1]

Re: [PATCH] btrfs-progs: Enhance read_tree_block to avoid memory corruption.

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH] btrfs-progs: Enhance read_tree_block to avoid 
memory corruption.
From: David Sterba <dsterba@suse.cz>
To: David Sterba <dsterba@suse.cz>
Date: 2015年05月21日 23:04

> On Thu, May 21, 2015 at 04:52:49PM +0200, David Sterba wrote:
>> The test 001-bad-file-extent-bytenr fails with this patch (and passes
>> otherwise). Can you please have a look?
>
> First check expectedly finds problems and fails, then repair is supposed
> to fix it, but the final check still finds problems because of the
> enhancements you've added:
>
> # output from the final check:
> checking extents
> invalid nr_items: 0
> Errors found in extent allocation tree or chunk allocation
> checking free space cache
> checking fs roots
> root 5 inode 257 errors 1000, some csum missing
> Checking filesystem on .../001-bad-file-extent-bytenr/default_case.img.restored
> UUID: 17442351-31aa-45fa-9503-90fd48874c3e
> cache and super generation don't match, space cache will be invalidated
> found 1081346 bytes used err is 1
> total csum bytes: 1024
> total tree bytes: 24576
> total fs tree bytes: 4096
> total extent tree bytes: 4096
> btree space waste bytes: 16507
> file data blocks allocated: 1310720
>   referenced 1310720
> btrfs-progs v4.0.1-2-gb7cf7c5c181d
> failed: .../btrfs check .../001-bad-file-extent-bytenr/default_case.img.restored
>
> It's "invalid nr_items: 0", while in the unpatched test there's
>
> Device extent[1, 29360128, 8388608] didn't find the relative chunk.
> Device extent[1, 1111490560, 1073741824] didn't find the relative chunk.
> Dev extent's total-byte(2185232384) is not equal to byte-used(1103101952) in dev[1, 216, 1]
>
Oh... I forgot the fact that empty tree is allowed to exist...
I'll update the patch soon.

Thanks
Qu