btrfs-progs: check: Avoid reading beyond item boundary for dir_item and dir_index

When reading out name from inode_ref, it's possible that corrupted
name_len can lead to read beyond boundary of item or even extent buffer.

This happens when checking fuzzed image /tmp/bko-161811.raw, for both
lowmem mode and original mode.

Below is the example from lowmem mode.

ERROR: root 5 INODE REF[256 256] doesn't have related DIR_INDEX[256 216172782113783808] namelen 255 filename bar filetype 0
ERROR: root 5 INODE REF[256 256] doesn't have related DIR_ITEM[256 1306590535] namelen 255 filename bar filetype 0
WARNING: root 5 INODE[256] mode 0 shouldn't have DIR_INDEX[256 1167283096]
WARNING: root 5 DIR_ITEM[256 1167283096] name too long
==13013== Invalid read of size 1
==13013==    at 0x4C31A38: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13013==    by 0x431518: read_extent_buffer (extent_io.c:863)
==13013==    by 0x4752AB: check_dir_item (cmds-check.c:4627)
==13013==    by 0x475E5C: check_inode_item (cmds-check.c:4911)
==13013==    by 0x476200: check_fs_first_inode (cmds-check.c:5011)
==13013==    by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13013==    by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13013==    by 0x488B5B: cmd_check (cmds-check.c:13033)
==13013==    by 0x40A8C5: main (btrfs.c:246)
==13013==  Address 0x5c95b80 is 0 bytes after a block of size 4,224 alloc'd
==13013==    at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13013==    by 0x4307E0: __alloc_extent_buffer (extent_io.c:538)
==13013==    by 0x430C37: alloc_extent_buffer (extent_io.c:642)
==13013==    by 0x413DFE: btrfs_find_create_tree_block (disk-io.c:193)
==13013==    by 0x414370: read_tree_block_fs_info (disk-io.c:340)
==13013==    by 0x40B5D5: read_tree_block (disk-io.h:125)
==13013==    by 0x40CFD2: read_node_slot (ctree.c:652)
==13013==    by 0x40E5EB: btrfs_search_slot (ctree.c:1172)
==13013==    by 0x4761A8: check_fs_first_inode (cmds-check.c:5001)
==13013==    by 0x476276: check_fs_root_v2 (cmds-check.c:5044)
==13013==    by 0x4769FB: check_fs_roots_v2 (cmds-check.c:5242)
==13013==    by 0x488B5B: cmd_check (cmds-check.c:13033)

Fix it by double checking dir_item, name_len against item boundary
before trying to read out name from extent buffer, for both original
mode and lowmem mode.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/cmds-check.c b/cmds-check.c
index 071af53..ad7c81b 100644 (file)
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -1512,13 +1512,19 @@ static int process_dir_item(struct extent_buffer *eb,
                filetype = btrfs_dir_type(eb, di);
 
                rec->found_size += name_len;
-               if (name_len <= BTRFS_NAME_LEN) {
+               if (cur + sizeof(*di) + name_len > total ||
+                   name_len > BTRFS_NAME_LEN) {
+                       error = REF_ERR_NAME_TOO_LONG;
+
+                       if (cur + sizeof(*di) > total)
+                               break;
+                       len = min_t(u32, total - cur - sizeof(*di),
+                                   BTRFS_NAME_LEN);
+               } else {
                        len = name_len;
                        error = 0;
-               } else {
-                       len = BTRFS_NAME_LEN;
-                       error = REF_ERR_NAME_TOO_LONG;
                }
+
                read_extent_buffer(eb, namebuf, (unsigned long)(di + 1), len);
 
                if (location.type == BTRFS_INODE_ITEM_KEY) {
@@ -4235,16 +4241,22 @@ static int find_dir_item(struct btrfs_root *root, struct btrfs_key *ref_key,
                if (imode_to_type(mode) != filetype)
                        goto next;
 
-               if (name_len <= BTRFS_NAME_LEN) {
-                       len = name_len;
-               } else {
-                       len = BTRFS_NAME_LEN;
+               if (cur + sizeof(*di) + name_len > total ||
+                   name_len > BTRFS_NAME_LEN) {
                        warning("root %llu %s[%llu %llu] name too long %u, trimmed",
-                       root->objectid,
-                       key->type == BTRFS_DIR_ITEM_KEY ?
-                       "DIR_ITEM" : "DIR_INDEX",
-                       key->objectid, key->offset, name_len);
+                               root->objectid,
+                               key->type == BTRFS_DIR_ITEM_KEY ?
+                               "DIR_ITEM" : "DIR_INDEX",
+                               key->objectid, key->offset, name_len);
+
+                       if (cur + sizeof(*di) > total)
+                               break;
+                       len = min_t(u32, total - cur - sizeof(*di),
+                                   BTRFS_NAME_LEN);
+               } else {
+                       len = name_len;
                }
+
                read_extent_buffer(node, namebuf, (unsigned long)(di + 1), len);
                if (len != namelen || strncmp(namebuf, name, len))
                        goto next;
@@ -4632,15 +4644,20 @@ static int check_dir_item(struct btrfs_root *root, struct btrfs_key *key,
                              key->objectid, key->offset, data_len);
 
                name_len = btrfs_dir_name_len(node, di);
-               if (name_len <= BTRFS_NAME_LEN) {
-                       len = name_len;
-               } else {
-                       len = BTRFS_NAME_LEN;
+               if (cur + sizeof(*di) + name_len > total ||
+                   name_len > BTRFS_NAME_LEN) {
                        warning("root %llu %s[%llu %llu] name too long",
                                root->objectid,
                                key->type == BTRFS_DIR_ITEM_KEY ?
                                "DIR_ITEM" : "DIR_INDEX",
                                key->objectid, key->offset);
+
+                       if (cur + sizeof(*di) > total)
+                               break;
+                       len = min_t(u32, total - cur - sizeof(*di),
+                                   BTRFS_NAME_LEN);
+               } else {
+                       len = name_len;
                }
                (*size) += name_len;