diff mbox series

btrfs: fix a use-after-free bug when hitting errors inside btrfs_submit_chunk()

Message ID 25987ba63d6e11a6983bf2c57eb2ac8efe059d8e.1723897285.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: fix a use-after-free bug when hitting errors inside btrfs_submit_chunk() | expand

Commit Message

Qu Wenruo Aug. 17, 2024, 12:21 p.m. UTC
[BUG]
There is an internal report that KASAN is reporting use-after-free, with
the following backtrace:

 ==================================================================
 BUG: KASAN: slab-use-after-free in btrfs_check_read_bio+0xa68/0xb70 [btrfs]
 Read of size 4 at addr ffff8881117cec28 by task kworker/u16:2/45
 CPU: 1 UID: 0 PID: 45 Comm: kworker/u16:2 Not tainted 6.11.0-rc2-next-20240805-default+ #76
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014
 Workqueue: btrfs-endio btrfs_end_bio_work [btrfs]
 Call Trace:
  <TASK>
  dump_stack_lvl+0x61/0x80
  print_address_description.constprop.0+0x5e/0x2f0
  print_report+0x118/0x216
  kasan_report+0x11d/0x1f0
  btrfs_check_read_bio+0xa68/0xb70 [btrfs]
  process_one_work+0xce0/0x12a0
  worker_thread+0x717/0x1250
  kthread+0x2e3/0x3c0
  ret_from_fork+0x2d/0x70
  ret_from_fork_asm+0x11/0x20
  </TASK>

 Allocated by task 20917:
  kasan_save_stack+0x37/0x60
  kasan_save_track+0x10/0x30
  __kasan_slab_alloc+0x7d/0x80
  kmem_cache_alloc_noprof+0x16e/0x3e0
  mempool_alloc_noprof+0x12e/0x310
  bio_alloc_bioset+0x3f0/0x7a0
  btrfs_bio_alloc+0x2e/0x50 [btrfs]
  submit_extent_page+0x4d1/0xdb0 [btrfs]
  btrfs_do_readpage+0x8b4/0x12a0 [btrfs]
  btrfs_readahead+0x29a/0x430 [btrfs]
  read_pages+0x1a7/0xc60
  page_cache_ra_unbounded+0x2ad/0x560
  filemap_get_pages+0x629/0xa20
  filemap_read+0x335/0xbf0
  vfs_read+0x790/0xcb0
  ksys_read+0xfd/0x1d0
  do_syscall_64+0x6d/0x140
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

 Freed by task 20917:
  kasan_save_stack+0x37/0x60
  kasan_save_track+0x10/0x30
  kasan_save_free_info+0x37/0x50
  __kasan_slab_free+0x4b/0x60
  kmem_cache_free+0x214/0x5d0
  bio_free+0xed/0x180
  end_bbio_data_read+0x1cc/0x580 [btrfs]
  btrfs_submit_chunk+0x98d/0x1880 [btrfs]
  btrfs_submit_bio+0x33/0x70 [btrfs]
  submit_one_bio+0xd4/0x130 [btrfs]
  submit_extent_page+0x3ea/0xdb0 [btrfs]
  btrfs_do_readpage+0x8b4/0x12a0 [btrfs]
  btrfs_readahead+0x29a/0x430 [btrfs]
  read_pages+0x1a7/0xc60
  page_cache_ra_unbounded+0x2ad/0x560
  filemap_get_pages+0x629/0xa20
  filemap_read+0x335/0xbf0
  vfs_read+0x790/0xcb0
  ksys_read+0xfd/0x1d0
  do_syscall_64+0x6d/0x140
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

[CAUSE]
Although I can not reproduce the error, the report itself is good enough
to pin down the cause.

The call trace is the regular endio workqueue context, but the
free-by-task trace is showing that during btrfs_submit_chunk() we
already hit a critical error, and is calling btrfs_bio_end_io() to error
out.
And the original endio function called bio_put() to free the whole bio.

This means a double freeing thus causing use-after-free, e.g:

1. Enter btrfs_submit_bio() with a read bio
   The read bio length is 128K, crossing two 64K stripes.

2. The first run of btrfs_submit_chunk()

2.1 Call btrfs_map_block(), which returns 64K
2.2 Call btrfs_split_bio()
    Now there are two bios, one referring to the first 64K, the other
    referring to the second 64K.
2.3 The first half is submitted.

3. The second run of btrfs_submit_chunk()

3.1 Call btrfs_map_block(), which by somehow failed
    Now we call btrfs_bio_end_io() to handle the error

3.2 btrfs_bio_end_io() calls the original endio function
    Which is end_bbio_data_read(), and it calls bio_put() for the
    original bio.

    Now the original bio is freed.

4. The submitted first 64K bio finished
   Now we call into btrfs_check_read_bio() and tries to advance the bio
   iter.
   But since the original bio (thus its iter) is already freed, we
   trigger the above use-after free.

   And even if the memory is not poisoned/corrupted, we will later call
   the original endio function, causing a double freeing.

[FIX]
Instead of calling btrfs_bio_end_io(), call btrfs_orig_bbio_end_io(),
which has the extra check on split bios and do the proper refcounting
for cloned bios.

Reported-by: David Sterba <dsterba@suse.cz>
Fixes: 852eee62d31a ("btrfs: allow btrfs_submit_bio to split bios")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/bio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

kernel test robot Aug. 17, 2024, 10:42 p.m. UTC | #1
Hi Qu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on linus/master v6.11-rc3 next-20240816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-fix-a-use-after-free-bug-when-hitting-errors-inside-btrfs_submit_chunk/20240817-202316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link:    https://lore.kernel.org/r/25987ba63d6e11a6983bf2c57eb2ac8efe059d8e.1723897285.git.wqu%40suse.com
patch subject: [PATCH] btrfs: fix a use-after-free bug when hitting errors inside btrfs_submit_chunk()
config: arc-randconfig-002-20240818 (https://download.01.org/0day-ci/archive/20240818/202408180546.vnwFs9i3-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240818/202408180546.vnwFs9i3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408180546.vnwFs9i3-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/btrfs/bio.c: In function 'btrfs_submit_chunk':
>> fs/btrfs/bio.c:671:27: warning: unused variable 'orig_bbio' [-Wunused-variable]
     671 |         struct btrfs_bio *orig_bbio = bbio;
         |                           ^~~~~~~~~


vim +/orig_bbio +671 fs/btrfs/bio.c

f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  666  
ae42a154ca89727 Christoph Hellwig  2023-03-07  667  static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
103c19723c80bf7 Christoph Hellwig  2022-11-15  668  {
d5e4377d505189c Christoph Hellwig  2023-01-21  669  	struct btrfs_inode *inode = bbio->inode;
4317ff0056bedfc Qu Wenruo          2023-03-23  670  	struct btrfs_fs_info *fs_info = bbio->fs_info;
852eee62d31abd6 Christoph Hellwig  2023-01-21 @671  	struct btrfs_bio *orig_bbio = bbio;
ae42a154ca89727 Christoph Hellwig  2023-03-07  672  	struct bio *bio = &bbio->bio;
adbe7e388e4239d Anand Jain         2023-04-15  673  	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
103c19723c80bf7 Christoph Hellwig  2022-11-15  674  	u64 length = bio->bi_iter.bi_size;
103c19723c80bf7 Christoph Hellwig  2022-11-15  675  	u64 map_length = length;
921603c76246a7f Christoph Hellwig  2022-12-12  676  	bool use_append = btrfs_use_zone_append(bbio);
103c19723c80bf7 Christoph Hellwig  2022-11-15  677  	struct btrfs_io_context *bioc = NULL;
103c19723c80bf7 Christoph Hellwig  2022-11-15  678  	struct btrfs_io_stripe smap;
9ba0004bd95e059 Christoph Hellwig  2023-01-21  679  	blk_status_t ret;
9ba0004bd95e059 Christoph Hellwig  2023-01-21  680  	int error;
103c19723c80bf7 Christoph Hellwig  2022-11-15  681  
31ae7881f9cd502 Johannes Thumshirn 2024-07-31  682  	if (!bbio->inode || btrfs_is_data_reloc_root(inode->root))
31ae7881f9cd502 Johannes Thumshirn 2024-07-31  683  		smap.rst_search_commit_root = true;
31ae7881f9cd502 Johannes Thumshirn 2024-07-31  684  	else
31ae7881f9cd502 Johannes Thumshirn 2024-07-31  685  		smap.rst_search_commit_root = false;
9acaa64187f9b4c Johannes Thumshirn 2023-09-14  686  
103c19723c80bf7 Christoph Hellwig  2022-11-15  687  	btrfs_bio_counter_inc_blocked(fs_info);
cd4efd210edfb34 Christoph Hellwig  2023-05-31  688  	error = btrfs_map_block(fs_info, btrfs_op(bio), logical, &map_length,
9fb2acc2fe07f15 Qu Wenruo          2023-09-17  689  				&bioc, &smap, &mirror_num);
9ba0004bd95e059 Christoph Hellwig  2023-01-21  690  	if (error) {
9ba0004bd95e059 Christoph Hellwig  2023-01-21  691  		ret = errno_to_blk_status(error);
9ba0004bd95e059 Christoph Hellwig  2023-01-21  692  		goto fail;
103c19723c80bf7 Christoph Hellwig  2022-11-15  693  	}
103c19723c80bf7 Christoph Hellwig  2022-11-15  694  
852eee62d31abd6 Christoph Hellwig  2023-01-21  695  	map_length = min(map_length, length);
d5e4377d505189c Christoph Hellwig  2023-01-21  696  	if (use_append)
d5e4377d505189c Christoph Hellwig  2023-01-21  697  		map_length = min(map_length, fs_info->max_zone_append_size);
d5e4377d505189c Christoph Hellwig  2023-01-21  698  
103c19723c80bf7 Christoph Hellwig  2022-11-15  699  	if (map_length < length) {
2cef0c79bb81d8b Christoph Hellwig  2023-03-07  700  		bbio = btrfs_split_bio(fs_info, bbio, map_length, use_append);
2cef0c79bb81d8b Christoph Hellwig  2023-03-07  701  		bio = &bbio->bio;
103c19723c80bf7 Christoph Hellwig  2022-11-15  702  	}
103c19723c80bf7 Christoph Hellwig  2022-11-15  703  
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  704  	/*
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  705  	 * Save the iter for the end_io handler and preload the checksums for
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  706  	 * data reads.
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  707  	 */
fbe960877b6f434 Christoph Hellwig  2023-05-31  708  	if (bio_op(bio) == REQ_OP_READ && is_data_bbio(bbio)) {
0d3acb25e70d5f5 Christoph Hellwig  2023-01-21  709  		bbio->saved_iter = bio->bi_iter;
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  710  		ret = btrfs_lookup_bio_sums(bbio);
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  711  		if (ret)
852eee62d31abd6 Christoph Hellwig  2023-01-21  712  			goto fail_put_bio;
1c2b3ee3b0ec4bc Christoph Hellwig  2023-01-21  713  	}
7276aa7d38255b4 Christoph Hellwig  2023-01-21  714  
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  715  	if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
d5e4377d505189c Christoph Hellwig  2023-01-21  716  		if (use_append) {
d5e4377d505189c Christoph Hellwig  2023-01-21  717  			bio->bi_opf &= ~REQ_OP_WRITE;
d5e4377d505189c Christoph Hellwig  2023-01-21  718  			bio->bi_opf |= REQ_OP_ZONE_APPEND;
69ccf3f4244abc5 Christoph Hellwig  2023-01-21  719  		}
69ccf3f4244abc5 Christoph Hellwig  2023-01-21  720  
02c372e1f016e51 Johannes Thumshirn 2023-09-14  721  		if (is_data_bbio(bbio) && bioc &&
02c372e1f016e51 Johannes Thumshirn 2023-09-14  722  		    btrfs_need_stripe_tree_update(bioc->fs_info, bioc->map_type)) {
02c372e1f016e51 Johannes Thumshirn 2023-09-14  723  			/*
02c372e1f016e51 Johannes Thumshirn 2023-09-14  724  			 * No locking for the list update, as we only add to
02c372e1f016e51 Johannes Thumshirn 2023-09-14  725  			 * the list in the I/O submission path, and list
02c372e1f016e51 Johannes Thumshirn 2023-09-14  726  			 * iteration only happens in the completion path, which
02c372e1f016e51 Johannes Thumshirn 2023-09-14  727  			 * can't happen until after the last submission.
02c372e1f016e51 Johannes Thumshirn 2023-09-14  728  			 */
02c372e1f016e51 Johannes Thumshirn 2023-09-14  729  			btrfs_get_bioc(bioc);
02c372e1f016e51 Johannes Thumshirn 2023-09-14  730  			list_add_tail(&bioc->rst_ordered_entry, &bbio->ordered->bioc_list);
02c372e1f016e51 Johannes Thumshirn 2023-09-14  731  		}
02c372e1f016e51 Johannes Thumshirn 2023-09-14  732  
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  733  		/*
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  734  		 * Csum items for reloc roots have already been cloned at this
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  735  		 * point, so they are handled as part of the no-checksum case.
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  736  		 */
4317ff0056bedfc Qu Wenruo          2023-03-23  737  		if (inode && !(inode->flags & BTRFS_INODE_NODATASUM) &&
169aaaf2e0be615 Qu Wenruo          2024-06-14  738  		    !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
d5e4377d505189c Christoph Hellwig  2023-01-21  739  		    !btrfs_is_data_reloc_root(inode->root)) {
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  740  			if (should_async_write(bbio) &&
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  741  			    btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
852eee62d31abd6 Christoph Hellwig  2023-01-21  742  				goto done;
103c19723c80bf7 Christoph Hellwig  2022-11-15  743  
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  744  			ret = btrfs_bio_csum(bbio);
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  745  			if (ret)
852eee62d31abd6 Christoph Hellwig  2023-01-21  746  				goto fail_put_bio;
cebae292e0c32a2 Johannes Thumshirn 2024-06-07  747  		} else if (use_append ||
cebae292e0c32a2 Johannes Thumshirn 2024-06-07  748  			   (btrfs_is_zoned(fs_info) && inode &&
cebae292e0c32a2 Johannes Thumshirn 2024-06-07  749  			    inode->flags & BTRFS_INODE_NODATASUM)) {
cbfce4c7fbde23c Christoph Hellwig  2023-05-24  750  			ret = btrfs_alloc_dummy_sum(bbio);
cbfce4c7fbde23c Christoph Hellwig  2023-05-24  751  			if (ret)
cbfce4c7fbde23c Christoph Hellwig  2023-05-24  752  				goto fail_put_bio;
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  753  		}
103c19723c80bf7 Christoph Hellwig  2022-11-15  754  	}
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  755  
f8a53bb58ec7e21 Christoph Hellwig  2023-01-21  756  	__btrfs_submit_bio(bio, bioc, &smap, mirror_num);
852eee62d31abd6 Christoph Hellwig  2023-01-21  757  done:
852eee62d31abd6 Christoph Hellwig  2023-01-21  758  	return map_length == length;
9ba0004bd95e059 Christoph Hellwig  2023-01-21  759  
852eee62d31abd6 Christoph Hellwig  2023-01-21  760  fail_put_bio:
852eee62d31abd6 Christoph Hellwig  2023-01-21  761  	if (map_length < length)
ec63b84d4611b2f Christoph Hellwig  2023-05-31  762  		btrfs_cleanup_bio(bbio);
9ba0004bd95e059 Christoph Hellwig  2023-01-21  763  fail:
9ba0004bd95e059 Christoph Hellwig  2023-01-21  764  	btrfs_bio_counter_dec(fs_info);
75f0d5c40c2c2f5 Qu Wenruo          2024-08-17  765  	bbio->bio.bi_status = ret;
75f0d5c40c2c2f5 Qu Wenruo          2024-08-17  766  	btrfs_orig_bbio_end_io(bbio);
852eee62d31abd6 Christoph Hellwig  2023-01-21  767  	/* Do not submit another chunk */
852eee62d31abd6 Christoph Hellwig  2023-01-21  768  	return true;
852eee62d31abd6 Christoph Hellwig  2023-01-21  769  }
852eee62d31abd6 Christoph Hellwig  2023-01-21  770
diff mbox series

Patch

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 0ef70fce85ff..4a0e97a56475 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -762,7 +762,8 @@  static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		btrfs_cleanup_bio(bbio);
 fail:
 	btrfs_bio_counter_dec(fs_info);
-	btrfs_bio_end_io(orig_bbio, ret);
+	bbio->bio.bi_status = ret;
+	btrfs_orig_bbio_end_io(bbio);
 	/* Do not submit another chunk */
 	return true;
 }