From 6a061158617f3aa670df861c912ef76d11aa69e4 Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Wed, 18 Dec 2019 09:19:39 +0800 Subject: [PATCH] btrfs-progs: disk-io: Verify the bytenr passed in is mapped for read_tree_block() [BUG] For a fuzzed image, `btrfs check` will segfault at open_ctree() stage: $ btrfs check --mode=lowmem issue_207.raw Opening filesystem to check... extent_io.c:665: free_extent_buffer_internal: BUG_ON `eb->refs < 0` triggered, value 1 btrfs(+0x6bf67)[0x56431d278f67] btrfs(+0x6c16e)[0x56431d27916e] btrfs(alloc_extent_buffer+0x45)[0x56431d279db5] btrfs(read_tree_block+0x59)[0x56431d2848f9] btrfs(btrfs_setup_all_roots+0x29c)[0x56431d28535c] btrfs(+0x78903)[0x56431d285903] btrfs(open_ctree_fs_info+0x90)[0x56431d285b60] btrfs(+0x45a01)[0x56431d252a01] btrfs(main+0x94)[0x56431d2220c4] /usr/lib/libc.so.6(__libc_start_main+0xf3)[0x7f6e28519153] btrfs(_start+0x2e)[0x56431d22235e] [CAUSE] The fuzzed image has a strange log root bytenr: log_root 61440 log_root_transid 0 In fact, the log_root seems to be fuzzed, as its transid is 0, which is invalid. Note that range [61440, 77824) covers the physical offset of the primary super block. The bug is caused by the following sequence: 1. cache for tree block [64K, 68K) is created by open_ctree() __open_ctree_fd() |- btrfs_setup_chunk_tree_and_device_map() |- btrfs_read_sys_array() |- sb = btrfs_find_create_tree_block() |- free_extent_buffer(sb) This created an extent buffer [64K, 68K) in fs_info->extent_cache, then reduce the refcount of that eb back to 0, but not freed yet. 2. Try to read that corrupted log root __open_ctree_fd() |- btrfs_setup_chunk_tree_and_device_map() |- btrfs_setup_all_roots() |- find_and_setup_log_root() |- read_tree_block() |- btrfs_find_create_tree_block() |- alloc_extent_buffer() The final alloc_extent_buffer() will try to free that cached eb [64K, 68K), since it doesn't match with current search. And since that cached eb is already released (refcount == 0), the extra free_extent_buffer() will cause above BUG_ON(). [FIX] Here we fix it through a more comprehensive method, instead of simply verifying log_root_transid, here we just don't pollute eb cache when reading sys chunk array. So that we won't have an eb cache [64K, 68K), and will error out at logical mapping phase. Issue: #207 Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- extent_io.c | 26 ++++++++++++++++++++++++++ extent_io.h | 2 ++ volumes.c | 3 ++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/extent_io.c b/extent_io.c index d4a68279..f11917a4 100644 --- a/extent_io.c +++ b/extent_io.c @@ -765,6 +765,32 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, return eb; } +/* + * Allocate a dummy extent buffer which won't be inserted into extent buffer + * cache. + * + * This mostly allows super block read write using existing eb infrastructure + * without pulluting the eb cache. + * + * This is especially important to avoid injecting eb->start == SZ_64K, as + * fuzzed image could have invalid tree bytenr covers super block range, + * and cause ref count underflow. + */ +struct extent_buffer *alloc_dummy_extent_buffer(struct btrfs_fs_info *fs_info, + u64 bytenr, u32 blocksize) +{ + struct extent_buffer *ret; + + ret = __alloc_extent_buffer(fs_info, bytenr, blocksize); + if (!ret) + return NULL; + + ret->tree = NULL; + ret->flags |= EXTENT_BUFFER_DUMMY; + + return ret; +} + int read_extent_from_disk(struct extent_buffer *eb, unsigned long offset, unsigned long len) { diff --git a/extent_io.h b/extent_io.h index 1715acc6..c6b8acf9 100644 --- a/extent_io.h +++ b/extent_io.h @@ -149,6 +149,8 @@ struct extent_buffer *find_first_extent_buffer(struct extent_io_tree *tree, struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 bytenr, u32 blocksize); struct extent_buffer *btrfs_clone_extent_buffer(struct extent_buffer *src); +struct extent_buffer *alloc_dummy_extent_buffer(struct btrfs_fs_info *fs_info, + u64 bytenr, u32 blocksize); void free_extent_buffer(struct extent_buffer *eb); void free_extent_buffer_nocache(struct extent_buffer *eb); int read_extent_from_disk(struct extent_buffer *eb, diff --git a/volumes.c b/volumes.c index 143164f0..44789f20 100644 --- a/volumes.c +++ b/volumes.c @@ -2144,7 +2144,8 @@ int btrfs_read_sys_array(struct btrfs_fs_info *fs_info) fs_info->nodesize); return -EINVAL; } - sb = btrfs_find_create_tree_block(fs_info, BTRFS_SUPER_INFO_OFFSET); + sb = alloc_dummy_extent_buffer(fs_info, BTRFS_SUPER_INFO_OFFSET, + BTRFS_SUPER_INFO_SIZE); if (!sb) return -ENOMEM; btrfs_set_buffer_uptodate(sb);