Skip to content

Commit d9bb77d

Browse files
adam900710kdave
authored andcommitted
btrfs: subpage: fix wild pointer access during metadata read failure
[BUG] When running fstests for btrfs subpage read-write test, it has a very high chance to crash at generic/475 with the following stack: BTRFS warning (device dm-8): direct IO failed ino 510 rw 1,34817 sector 0xcdf0 len 94208 err no 10 Unable to handle kernel paging request at virtual address ffff80001157e7c0 CPU: 2 PID: 687125 Comm: kworker/u12:4 Tainted: G WC 5.12.0-rc2-custom+ #5 Hardware name: Khadas VIM3 (DT) Workqueue: btrfs-endio-meta btrfs_work_helper [btrfs] pc : queued_spin_lock_slowpath+0x1a0/0x390 lr : do_raw_spin_lock+0xc4/0x11c Call trace: queued_spin_lock_slowpath+0x1a0/0x390 _raw_spin_lock+0x68/0x84 btree_readahead_hook+0x38/0xc0 [btrfs] end_bio_extent_readpage+0x504/0x5f4 [btrfs] bio_endio+0x170/0x1a4 end_workqueue_fn+0x3c/0x60 [btrfs] btrfs_work_helper+0x1b0/0x1b4 [btrfs] process_one_work+0x22c/0x430 worker_thread+0x70/0x3a0 kthread+0x13c/0x140 ret_from_fork+0x10/0x30 Code: 910020e0 8b0200c2 f861d884 aa0203e1 (f8246827) [CAUSE] In end_bio_extent_readpage(), if we hit an error during read, we will handle the error differently for data and metadata. For data we queue a repair, while for metadata, we record the error and let the caller choose what to do. But the code is still using page->private to grab extent buffer, which no longer points to extent buffer for subpage metadata pages. Thus this wild pointer access leads to above crash. [FIX] Introduce a helper, find_extent_buffer_readpage(), to grab extent buffer. The difference against find_extent_buffer_nospinlock() is: - Also handles regular sectorsize == PAGE_SIZE case - No extent buffer refs increase/decrease As extent buffer under IO must have non-zero refs, so this is safe Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent e3d3b41 commit d9bb77d

File tree

1 file changed

+30
-1
lines changed

1 file changed

+30
-1
lines changed

fs/btrfs/extent_io.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2885,6 +2885,35 @@ static void end_page_read(struct page *page, bool uptodate, u64 start, u32 len)
28852885
btrfs_subpage_end_reader(fs_info, page, start, len);
28862886
}
28872887

2888+
/*
2889+
* Find extent buffer for a givne bytenr.
2890+
*
2891+
* This is for end_bio_extent_readpage(), thus we can't do any unsafe locking
2892+
* in endio context.
2893+
*/
2894+
static struct extent_buffer *find_extent_buffer_readpage(
2895+
struct btrfs_fs_info *fs_info, struct page *page, u64 bytenr)
2896+
{
2897+
struct extent_buffer *eb;
2898+
2899+
/*
2900+
* For regular sectorsize, we can use page->private to grab extent
2901+
* buffer
2902+
*/
2903+
if (fs_info->sectorsize == PAGE_SIZE) {
2904+
ASSERT(PagePrivate(page) && page->private);
2905+
return (struct extent_buffer *)page->private;
2906+
}
2907+
2908+
/* For subpage case, we need to lookup buffer radix tree */
2909+
rcu_read_lock();
2910+
eb = radix_tree_lookup(&fs_info->buffer_radix,
2911+
bytenr >> fs_info->sectorsize_bits);
2912+
rcu_read_unlock();
2913+
ASSERT(eb);
2914+
return eb;
2915+
}
2916+
28882917
/*
28892918
* after a readpage IO is done, we need to:
28902919
* clear the uptodate bits on error
@@ -2996,7 +3025,7 @@ static void end_bio_extent_readpage(struct bio *bio)
29963025
} else {
29973026
struct extent_buffer *eb;
29983027

2999-
eb = (struct extent_buffer *)page->private;
3028+
eb = find_extent_buffer_readpage(fs_info, page, start);
30003029
set_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
30013030
eb->read_mirror = mirror;
30023031
atomic_dec(&eb->io_pages);

0 commit comments

Comments
 (0)