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

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

[Qu Wenruo]

[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.

Furthermore there is already one extra btrfs_cleanup_bio() call, but
that is duplicated to btrfs_orig_bbio_end_io() call, so remove that
tag completely.

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>
---
Changelog:
v4:
- Fix a case where the endio function is never called
  If we have split the bbio and hit a critical error, we have to end
  both the current and the remaining bbio.
  As the remaining one will never be submitted, thus pending_ios will
  never reach 0.

v3:
- Fix a double free if we go into fail_put_bio() tag
  By removing the tag and the btrfs_cleanup_bio() call completely.

v2:
- Remove the unused variable  @orig_bbio
---
 fs/btrfs/bio.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 0ef70fce85ff..f6cb58d7f16a 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -668,7 +668,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 {
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = bbio->fs_info;
-	struct btrfs_bio *orig_bbio = bbio;
 	struct bio *bio = &bbio->bio;
 	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	u64 length = bio->bi_iter.bi_size;
@@ -709,7 +708,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		bbio->saved_iter = bio->bi_iter;
 		ret = btrfs_lookup_bio_sums(bbio);
 		if (ret)
-			goto fail_put_bio;
+			goto fail;
 	}
 
 	if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
@@ -743,13 +742,13 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 
 			ret = btrfs_bio_csum(bbio);
 			if (ret)
-				goto fail_put_bio;
+				goto fail;
 		} else if (use_append ||
 			   (btrfs_is_zoned(fs_info) && inode &&
 			    inode->flags & BTRFS_INODE_NODATASUM)) {
 			ret = btrfs_alloc_dummy_sum(bbio);
 			if (ret)
-				goto fail_put_bio;
+				goto fail;
 		}
 	}
 
@@ -757,12 +756,23 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 done:
 	return map_length == length;
 
-fail_put_bio:
-	if (map_length < length)
-		btrfs_cleanup_bio(bbio);
 fail:
 	btrfs_bio_counter_dec(fs_info);
-	btrfs_bio_end_io(orig_bbio, ret);
+	/*
+	 * We have split the original bbio, now we have to end both the current
+	 * @bbio and remaining one, as the remaining one will never be submitted.
+	 */
+	if (map_length < length) {
+		struct btrfs_bio *remaining = bbio->private;
+
+		ASSERT(bbio->bio.bi_pool == &btrfs_clone_bioset);
+		ASSERT(remaining);
+
+		remaining->bio.bi_status = ret;
+		btrfs_orig_bbio_end_io(remaining);
+	}
+	bbio->bio.bi_status = ret;
+	btrfs_orig_bbio_end_io(bbio);
 	/* Do not submit another chunk */
 	return true;
 }
-- 
2.46.0

Re: [PATCH v4] btrfs: fix a use-after-free bug when hitting errors inside btrfs_submit_chunk()

[Josef Bacik]

On Sat, Aug 24, 2024 at 07:28:23PM +0930, Qu Wenruo wrote:
> [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.
> 
> Furthermore there is already one extra btrfs_cleanup_bio() call, but
> that is duplicated to btrfs_orig_bbio_end_io() call, so remove that
> tag completely.
> 
> 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>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH v4] btrfs: fix a use-after-free bug when hitting errors inside btrfs_submit_chunk()

[David Sterba]

On Sat, Aug 24, 2024 at 07:28:23PM +0930, Qu Wenruo wrote:
> [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.
> 
> Furthermore there is already one extra btrfs_cleanup_bio() call, but
> that is duplicated to btrfs_orig_bbio_end_io() call, so remove that
> tag completely.
> 
> 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>
> ---
> Changelog:
> v4:
> - Fix a case where the endio function is never called
>   If we have split the bbio and hit a critical error, we have to end
>   both the current and the remaining bbio.
>   As the remaining one will never be submitted, thus pending_ios will
>   never reach 0.

The tests now pass, thanks.

Reviewed-by: David Sterba <dsterba@suse.com>