| Message ID | 20190313085511.23540-4-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Enhance tree checker and runtime checker to handle the new wave of fuzzed image attack | expand |
On 13.03.19 г. 10:55 ч., Qu Wenruo wrote: > [BUG] > When reading a file from a fuzzed image, kernel can panic like: > BTRFS warning (device loop0): csum failed root 5 ino 270 off 0 csum 0x98f94189 expected csum 0x00000000 mirror 1 > assertion failed: !memcmp_extent_buffer(b, &disk_key, offsetof(struct btrfs_leaf, items[0].key), sizeof(disk_key)), file: fs/btrfs/ctree.c, line: 2544 > ------------[ cut here ]------------ > kernel BUG at fs/btrfs/ctree.h:3500! > invalid opcode: 0000 [#1] PREEMPT SMP NOPTI > RIP: 0010:btrfs_search_slot.cold.24+0x61/0x63 [btrfs] > Call Trace: > btrfs_lookup_csum+0x52/0x150 [btrfs] > __btrfs_lookup_bio_sums+0x209/0x640 [btrfs] > btrfs_submit_bio_hook+0x103/0x170 [btrfs] > submit_one_bio+0x59/0x80 [btrfs] > extent_read_full_page+0x58/0x80 [btrfs] > generic_file_read_iter+0x2f6/0x9d0 > __vfs_read+0x14d/0x1a0 > vfs_read+0x8d/0x140 > ksys_read+0x52/0xc0 > do_syscall_64+0x60/0x210 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > [CAUSE] > The fuzzed image has a corrupted leaf whose first key doesn't match with its parent: > checksum tree key (CSUM_TREE ROOT_ITEM 0) > node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE > fs uuid 3381d111-94a3-4ac7-8f39-611bbbdab7e6 > chunk uuid 9af1c3c7-2af5-488b-8553-530bd515f14c > ... > key (EXTENT_CSUM EXTENT_CSUM 79691776) block 29761536 gen 19 > > leaf 29761536 items 1 free space 1726 generation 19 owner CSUM_TREE > leaf 29761536 flags 0x1(WRITTEN) backref revision 1 > fs uuid 3381d111-94a3-4ac7-8f39-611bbbdab7e6 > chunk uuid 9af1c3c7-2af5-488b-8553-530bd515f14c > item 0 key (EXTENT_CSUM EXTENT_CSUM 8798638964736) itemoff 1751 itemsize 2244 > range start 8798638964736 end 8798641262592 length 2297856 > > When reading above tree block, we have extent_buffer->refs = 2 in the > context: > - initial one from __alloc_extent_buffer() > alloc_extent_buffer() > |- __alloc_extent_buffer() > |- atomic_set(&eb->refs, 1) > > - one being added to fs_info->buffer_radix > alloc_extent_buffer() > |- check_buffer_tree_ref() > |- atomic_inc(&eb->refs) > > So even we call free_extent_buffer() in read_tree_block or other similar > situation, we only decrease the refs by 1, it doesn't reach 0 and won't > be freed right now. > > The staled eb and its corrupted content will still be kept cached. > > Further more, we have several extra cases where we either don't do > first key check or the check is not proper for all callers: > - scrub > We just don't have first key in this context. > > - shared tree block > One tree block can be shared by several snapshot/subvolume trees. > In that case, the first key check for one subvolume doesn't apply to > another. > > So for above reasons, a corrupted extent buffer can sneak into the > buffer cache. > > [FIX] > Export verify_level_key() as btrfs_verify_level_key() and call it in > read_block_for_search() to fill the hole. > > Due to above described reasons, even we can free corrupted extent buffer > from cache, we still need the check in read_block_for_search(), for > scrub and shared tree blocks. > > Reported-by: Yoon Jungyeon <jungyeon@gatech.edu> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202755 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202757 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202759 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202761 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202767 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202769 > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- I had already reviewed the earlier posting of this patch so: Reviewed-by: Nikolay Borisov <nborisov@suse.com> > fs/btrfs/ctree.c | 10 ++++++++++ > fs/btrfs/disk-io.c | 10 +++++----- > fs/btrfs/disk-io.h | 3 +++ > 3 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 5a6c39b44c84..7672932aa5b4 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -2401,6 +2401,16 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p, > if (tmp) { > /* first we do an atomic uptodate check */ > if (btrfs_buffer_uptodate(tmp, gen, 1) > 0) { > + /* > + * Do extra check for first_key, eb can be stale due to > + * being cached, read from scrub, or have multiple > + * parents (shared tree blocks). > + */ > + if (btrfs_verify_level_key(fs_info, tmp, > + parent_level - 1, &first_key, gen)) { > + free_extent_buffer(tmp); > + return -EUCLEAN; > + } > *eb_ret = tmp; > return 0; > } > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 298b34721bc0..e2a0cb362d28 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -423,9 +423,9 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info, > return ret; > } > > -static int verify_level_key(struct btrfs_fs_info *fs_info, > - struct extent_buffer *eb, int level, > - struct btrfs_key *first_key, u64 parent_transid) > +int btrfs_verify_level_key(struct btrfs_fs_info *fs_info, > + struct extent_buffer *eb, int level, > + struct btrfs_key *first_key, u64 parent_transid) > { > int found_level; > struct btrfs_key found_key; > @@ -500,8 +500,8 @@ static int btree_read_extent_buffer_pages(struct btrfs_fs_info *fs_info, > if (verify_parent_transid(io_tree, eb, > parent_transid, 0)) > ret = -EIO; > - else if (verify_level_key(fs_info, eb, level, > - first_key, parent_transid)) > + else if (btrfs_verify_level_key(fs_info, eb, level, > + first_key, parent_transid)) > ret = -EUCLEAN; > else > break; > diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h > index 987a64bc0c66..67a9fe2d29c7 100644 > --- a/fs/btrfs/disk-io.h > +++ b/fs/btrfs/disk-io.h > @@ -39,6 +39,9 @@ static inline u64 btrfs_sb_offset(int mirror) > struct btrfs_device; > struct btrfs_fs_devices; > > +int btrfs_verify_level_key(struct btrfs_fs_info *fs_info, > + struct extent_buffer *eb, int level, > + struct btrfs_key *first_key, u64 parent_transid); > struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr, > u64 parent_transid, int level, > struct btrfs_key *first_key); >
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 5a6c39b44c84..7672932aa5b4 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -2401,6 +2401,16 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p, if (tmp) { /* first we do an atomic uptodate check */ if (btrfs_buffer_uptodate(tmp, gen, 1) > 0) { + /* + * Do extra check for first_key, eb can be stale due to + * being cached, read from scrub, or have multiple + * parents (shared tree blocks). + */ + if (btrfs_verify_level_key(fs_info, tmp, + parent_level - 1, &first_key, gen)) { + free_extent_buffer(tmp); + return -EUCLEAN; + } *eb_ret = tmp; return 0; } diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 298b34721bc0..e2a0cb362d28 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -423,9 +423,9 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info, return ret; } -static int verify_level_key(struct btrfs_fs_info *fs_info, - struct extent_buffer *eb, int level, - struct btrfs_key *first_key, u64 parent_transid) +int btrfs_verify_level_key(struct btrfs_fs_info *fs_info, + struct extent_buffer *eb, int level, + struct btrfs_key *first_key, u64 parent_transid) { int found_level; struct btrfs_key found_key; @@ -500,8 +500,8 @@ static int btree_read_extent_buffer_pages(struct btrfs_fs_info *fs_info, if (verify_parent_transid(io_tree, eb, parent_transid, 0)) ret = -EIO; - else if (verify_level_key(fs_info, eb, level, - first_key, parent_transid)) + else if (btrfs_verify_level_key(fs_info, eb, level, + first_key, parent_transid)) ret = -EUCLEAN; else break; diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h index 987a64bc0c66..67a9fe2d29c7 100644 --- a/fs/btrfs/disk-io.h +++ b/fs/btrfs/disk-io.h @@ -39,6 +39,9 @@ static inline u64 btrfs_sb_offset(int mirror) struct btrfs_device; struct btrfs_fs_devices; +int btrfs_verify_level_key(struct btrfs_fs_info *fs_info, + struct extent_buffer *eb, int level, + struct btrfs_key *first_key, u64 parent_transid); struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr, u64 parent_transid, int level, struct btrfs_key *first_key);
[BUG] When reading a file from a fuzzed image, kernel can panic like: BTRFS warning (device loop0): csum failed root 5 ino 270 off 0 csum 0x98f94189 expected csum 0x00000000 mirror 1 assertion failed: !memcmp_extent_buffer(b, &disk_key, offsetof(struct btrfs_leaf, items[0].key), sizeof(disk_key)), file: fs/btrfs/ctree.c, line: 2544 ------------[ cut here ]------------ kernel BUG at fs/btrfs/ctree.h:3500! invalid opcode: 0000 [#1] PREEMPT SMP NOPTI RIP: 0010:btrfs_search_slot.cold.24+0x61/0x63 [btrfs] Call Trace: btrfs_lookup_csum+0x52/0x150 [btrfs] __btrfs_lookup_bio_sums+0x209/0x640 [btrfs] btrfs_submit_bio_hook+0x103/0x170 [btrfs] submit_one_bio+0x59/0x80 [btrfs] extent_read_full_page+0x58/0x80 [btrfs] generic_file_read_iter+0x2f6/0x9d0 __vfs_read+0x14d/0x1a0 vfs_read+0x8d/0x140 ksys_read+0x52/0xc0 do_syscall_64+0x60/0x210 entry_SYSCALL_64_after_hwframe+0x49/0xbe [CAUSE] The fuzzed image has a corrupted leaf whose first key doesn't match with its parent: checksum tree key (CSUM_TREE ROOT_ITEM 0) node 29741056 level 1 items 14 free 107 generation 19 owner CSUM_TREE fs uuid 3381d111-94a3-4ac7-8f39-611bbbdab7e6 chunk uuid 9af1c3c7-2af5-488b-8553-530bd515f14c ... key (EXTENT_CSUM EXTENT_CSUM 79691776) block 29761536 gen 19 leaf 29761536 items 1 free space 1726 generation 19 owner CSUM_TREE leaf 29761536 flags 0x1(WRITTEN) backref revision 1 fs uuid 3381d111-94a3-4ac7-8f39-611bbbdab7e6 chunk uuid 9af1c3c7-2af5-488b-8553-530bd515f14c item 0 key (EXTENT_CSUM EXTENT_CSUM 8798638964736) itemoff 1751 itemsize 2244 range start 8798638964736 end 8798641262592 length 2297856 When reading above tree block, we have extent_buffer->refs = 2 in the context: - initial one from __alloc_extent_buffer() alloc_extent_buffer() |- __alloc_extent_buffer() |- atomic_set(&eb->refs, 1) - one being added to fs_info->buffer_radix alloc_extent_buffer() |- check_buffer_tree_ref() |- atomic_inc(&eb->refs) So even we call free_extent_buffer() in read_tree_block or other similar situation, we only decrease the refs by 1, it doesn't reach 0 and won't be freed right now. The staled eb and its corrupted content will still be kept cached. Further more, we have several extra cases where we either don't do first key check or the check is not proper for all callers: - scrub We just don't have first key in this context. - shared tree block One tree block can be shared by several snapshot/subvolume trees. In that case, the first key check for one subvolume doesn't apply to another. So for above reasons, a corrupted extent buffer can sneak into the buffer cache. [FIX] Export verify_level_key() as btrfs_verify_level_key() and call it in read_block_for_search() to fill the hole. Due to above described reasons, even we can free corrupted extent buffer from cache, we still need the check in read_block_for_search(), for scrub and shared tree blocks. Reported-by: Yoon Jungyeon <jungyeon@gatech.edu> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202755 Link: https://bugzilla.kernel.org/show_bug.cgi?id=202757 Link: https://bugzilla.kernel.org/show_bug.cgi?id=202759 Link: https://bugzilla.kernel.org/show_bug.cgi?id=202761 Link: https://bugzilla.kernel.org/show_bug.cgi?id=202767 Link: https://bugzilla.kernel.org/show_bug.cgi?id=202769 Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/ctree.c | 10 ++++++++++ fs/btrfs/disk-io.c | 10 +++++----- fs/btrfs/disk-io.h | 3 +++ 3 files changed, 18 insertions(+), 5 deletions(-)