[PATCH] fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate()

[PATCH] fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate()

[Zixuan Fu]

In btrfs_relocate_block_group(), the structure variable rc is allocated.
Then btrfs_relocate_block_group() calls relocate_block_group() -> 
prepare_to_relocate() -> set_reloc_control(), and assigns rc to the
variable fs_info->reloc_ctl. When prepare_to_relocate() returns, it
calls btrfs_commit_transaction() -> btrfs_start_dirty_block_groups()
-> btrfs_alloc_path() -> kmem_cache_zalloc(), which may fail. When the
failure occurs, btrfs_relocate_block_group() detects the error and frees
rc and doesn't set fs_info->reloc_ctl to NULL. After that, in 
btrfs_init_reloc_root(), rc is retrieved from fs_info->reloc_ctl and
then used, which may cause a use-after-free bug.

This possible bug can be triggered by calling btrfs_ioctl_balance()
before calling btrfs_ioctl_defrag().

To fix this possible bug, in prepare_to_relocate(), an if statement
is added to check whether btrfs_commit_transaction() fails. If the
failure occurs, unset_reloc_control() is called to set
fs_info->reloc_ctl to NULL.

The error log in our fault-injection testing is shown as follows:

[   58.751070] BUG: KASAN: use-after-free in btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
...
[   58.753577] Call Trace:
...
[   58.755800]  kasan_report+0x45/0x60
[   58.756066]  btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
[   58.757304]  record_root_in_trans+0x792/0xa10 [btrfs]
[   58.757748]  btrfs_record_root_in_trans+0x463/0x4f0 [btrfs]
[   58.758231]  start_transaction+0x896/0x2950 [btrfs]
[   58.758661]  btrfs_defrag_root+0x250/0xc00 [btrfs]
[   58.759083]  btrfs_ioctl_defrag+0x467/0xa00 [btrfs]
[   58.759513]  btrfs_ioctl+0x3c95/0x114e0 [btrfs]
...
[   58.768510] Allocated by task 23683:
[   58.768777]  ____kasan_kmalloc+0xb5/0xf0
[   58.769069]  __kmalloc+0x227/0x3d0
[   58.769325]  alloc_reloc_control+0x10a/0x3d0 [btrfs]
[   58.769755]  btrfs_relocate_block_group+0x7aa/0x1e20 [btrfs]
[   58.770228]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
[   58.770655]  __btrfs_balance+0x1326/0x1f10 [btrfs]
[   58.771071]  btrfs_balance+0x3150/0x3d30 [btrfs]
[   58.771472]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
[   58.771902]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
...
[   58.773337] Freed by task 23683:
...
[   58.774815]  kfree+0xda/0x2b0
[   58.775038]  free_reloc_control+0x1d6/0x220 [btrfs]
[   58.775465]  btrfs_relocate_block_group+0x115c/0x1e20 [btrfs]
[   58.775944]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
[   58.776369]  __btrfs_balance+0x1326/0x1f10 [btrfs]
[   58.776784]  btrfs_balance+0x3150/0x3d30 [btrfs]
[   58.777185]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
[   58.777621]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
...

Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
Signed-off-by: Zixuan Fu <r33s3n6@gmail.com>
---
 fs/btrfs/relocation.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index a6dc827e75af..342ade83d062 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3573,7 +3573,12 @@ int prepare_to_relocate(struct reloc_control *rc)
 		 */
 		return PTR_ERR(trans);
 	}
-	return btrfs_commit_transaction(trans);
+
+	ret = btrfs_commit_transaction(trans);
+	if (ret)
+		unset_reloc_control(rc);
+
+	return ret;
 }
 
 static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
-- 
2.25.1

Re: [PATCH] fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate()

[Sweet Tea Dorminy]

On 7/21/22 03:48, Zixuan Fu wrote:
> In btrfs_relocate_block_group(), the structure variable rc is allocated.
> Then btrfs_relocate_block_group() calls relocate_block_group() ->
> prepare_to_relocate() -> set_reloc_control(), and assigns rc to the
> variable fs_info->reloc_ctl. When prepare_to_relocate() returns, it
> calls btrfs_commit_transaction() -> btrfs_start_dirty_block_groups()
> -> btrfs_alloc_path() -> kmem_cache_zalloc(), which may fail. When the
> failure occurs, btrfs_relocate_block_group() detects the error and frees
> rc and doesn't set fs_info->reloc_ctl to NULL. After that, in
> btrfs_init_reloc_root(), rc is retrieved from fs_info->reloc_ctl and
> then used, which may cause a use-after-free bug.
> 
> This possible bug can be triggered by calling btrfs_ioctl_balance()
> before calling btrfs_ioctl_defrag().
> 
> To fix this possible bug, in prepare_to_relocate(), an if statement
> is added to check whether btrfs_commit_transaction() fails. If the
> failure occurs, unset_reloc_control() is called to set
> fs_info->reloc_ctl to NULL.
> 
> The error log in our fault-injection testing is shown as follows:
> 
> [   58.751070] BUG: KASAN: use-after-free in btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> ...
> [   58.753577] Call Trace:
> ...
> [   58.755800]  kasan_report+0x45/0x60
> [   58.756066]  btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> [   58.757304]  record_root_in_trans+0x792/0xa10 [btrfs]
> [   58.757748]  btrfs_record_root_in_trans+0x463/0x4f0 [btrfs]
> [   58.758231]  start_transaction+0x896/0x2950 [btrfs]
> [   58.758661]  btrfs_defrag_root+0x250/0xc00 [btrfs]
> [   58.759083]  btrfs_ioctl_defrag+0x467/0xa00 [btrfs]
> [   58.759513]  btrfs_ioctl+0x3c95/0x114e0 [btrfs]
> ...
> [   58.768510] Allocated by task 23683:
> [   58.768777]  ____kasan_kmalloc+0xb5/0xf0
> [   58.769069]  __kmalloc+0x227/0x3d0
> [   58.769325]  alloc_reloc_control+0x10a/0x3d0 [btrfs]
> [   58.769755]  btrfs_relocate_block_group+0x7aa/0x1e20 [btrfs]
> [   58.770228]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.770655]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.771071]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.771472]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.771902]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> [   58.773337] Freed by task 23683:
> ...
> [   58.774815]  kfree+0xda/0x2b0
> [   58.775038]  free_reloc_control+0x1d6/0x220 [btrfs]
> [   58.775465]  btrfs_relocate_block_group+0x115c/0x1e20 [btrfs]
> [   58.775944]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.776369]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.776784]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.777185]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.777621]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> 
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Zixuan Fu <r33s3n6@gmail.com>
Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>   fs/btrfs/relocation.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index a6dc827e75af..342ade83d062 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -3573,7 +3573,12 @@ int prepare_to_relocate(struct reloc_control *rc)
>   		 */
>   		return PTR_ERR(trans);
>   	}
> -	return btrfs_commit_transaction(trans);
> +
> +	ret = btrfs_commit_transaction(trans);
> +	if (ret)
> +		unset_reloc_control(rc);
> +
> +	return ret;
>   }
>   
>   static noinline_for_stack int relocate_block_group(struct reloc_control *rc)

Re: [PATCH] fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate()

[Nikolay Borisov]

On 21.07.22 г. 10:48 ч., Zixuan Fu wrote:
> In btrfs_relocate_block_group(), the structure variable rc is allocated.
> Then btrfs_relocate_block_group() calls relocate_block_group() ->
> prepare_to_relocate() -> set_reloc_control(), and assigns rc to the
> variable fs_info->reloc_ctl. When prepare_to_relocate() returns, it
> calls btrfs_commit_transaction() -> btrfs_start_dirty_block_groups()

nit: transaction commit can fail for a number of reasons, not 
necessarily enomem, even due to ENOSPC errors or faulty hardware.

> -> btrfs_alloc_path() -> kmem_cache_zalloc(), which may fail. When the
> failure occurs, btrfs_relocate_block_group() detects the error and frees
> rc and doesn't set fs_info->reloc_ctl to NULL. After that, in
> btrfs_init_reloc_root(), rc is retrieved from fs_info->reloc_ctl and
> then used, which may cause a use-after-free bug.
> 
> This possible bug can be triggered by calling btrfs_ioctl_balance()
> before calling btrfs_ioctl_defrag().
> 
> To fix this possible bug, in prepare_to_relocate(), an if statement
> is added to check whether btrfs_commit_transaction() fails. If the
> failure occurs, unset_reloc_control() is called to set
> fs_info->reloc_ctl to NULL.
> 
> The error log in our fault-injection testing is shown as follows:
> 
> [   58.751070] BUG: KASAN: use-after-free in btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> ...
> [   58.753577] Call Trace:
> ...
> [   58.755800]  kasan_report+0x45/0x60
> [   58.756066]  btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> [   58.757304]  record_root_in_trans+0x792/0xa10 [btrfs]
> [   58.757748]  btrfs_record_root_in_trans+0x463/0x4f0 [btrfs]
> [   58.758231]  start_transaction+0x896/0x2950 [btrfs]
> [   58.758661]  btrfs_defrag_root+0x250/0xc00 [btrfs]
> [   58.759083]  btrfs_ioctl_defrag+0x467/0xa00 [btrfs]
> [   58.759513]  btrfs_ioctl+0x3c95/0x114e0 [btrfs]
> ...
> [   58.768510] Allocated by task 23683:
> [   58.768777]  ____kasan_kmalloc+0xb5/0xf0
> [   58.769069]  __kmalloc+0x227/0x3d0
> [   58.769325]  alloc_reloc_control+0x10a/0x3d0 [btrfs]
> [   58.769755]  btrfs_relocate_block_group+0x7aa/0x1e20 [btrfs]
> [   58.770228]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.770655]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.771071]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.771472]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.771902]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> [   58.773337] Freed by task 23683:
> ...
> [   58.774815]  kfree+0xda/0x2b0
> [   58.775038]  free_reloc_control+0x1d6/0x220 [btrfs]
> [   58.775465]  btrfs_relocate_block_group+0x115c/0x1e20 [btrfs]
> [   58.775944]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.776369]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.776784]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.777185]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.777621]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> 
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Zixuan Fu <r33s3n6@gmail.com>


Reviewed-by: Nikolay Borisov <nborisov@suse.com>

Re: [PATCH] fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate()

[David Sterba]

On Thu, Jul 21, 2022 at 03:48:29PM +0800, Zixuan Fu wrote:
> In btrfs_relocate_block_group(), the structure variable rc is allocated.
> Then btrfs_relocate_block_group() calls relocate_block_group() -> 
> prepare_to_relocate() -> set_reloc_control(), and assigns rc to the
> variable fs_info->reloc_ctl. When prepare_to_relocate() returns, it
> calls btrfs_commit_transaction() -> btrfs_start_dirty_block_groups()
> -> btrfs_alloc_path() -> kmem_cache_zalloc(), which may fail. When the
> failure occurs, btrfs_relocate_block_group() detects the error and frees
> rc and doesn't set fs_info->reloc_ctl to NULL. After that, in 
> btrfs_init_reloc_root(), rc is retrieved from fs_info->reloc_ctl and
> then used, which may cause a use-after-free bug.

I've reformatted the paragraph, the call chains are otherwise
incomprehensible.

> This possible bug can be triggered by calling btrfs_ioctl_balance()
> before calling btrfs_ioctl_defrag().
> 
> To fix this possible bug, in prepare_to_relocate(), an if statement
> is added to check whether btrfs_commit_transaction() fails. If the
> failure occurs, unset_reloc_control() is called to set
> fs_info->reloc_ctl to NULL.
> 
> The error log in our fault-injection testing is shown as follows:
> 
> [   58.751070] BUG: KASAN: use-after-free in btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> ...
> [   58.753577] Call Trace:
> ...
> [   58.755800]  kasan_report+0x45/0x60
> [   58.756066]  btrfs_init_reloc_root+0x7ca/0x920 [btrfs]
> [   58.757304]  record_root_in_trans+0x792/0xa10 [btrfs]
> [   58.757748]  btrfs_record_root_in_trans+0x463/0x4f0 [btrfs]
> [   58.758231]  start_transaction+0x896/0x2950 [btrfs]
> [   58.758661]  btrfs_defrag_root+0x250/0xc00 [btrfs]
> [   58.759083]  btrfs_ioctl_defrag+0x467/0xa00 [btrfs]
> [   58.759513]  btrfs_ioctl+0x3c95/0x114e0 [btrfs]
> ...
> [   58.768510] Allocated by task 23683:
> [   58.768777]  ____kasan_kmalloc+0xb5/0xf0
> [   58.769069]  __kmalloc+0x227/0x3d0
> [   58.769325]  alloc_reloc_control+0x10a/0x3d0 [btrfs]
> [   58.769755]  btrfs_relocate_block_group+0x7aa/0x1e20 [btrfs]
> [   58.770228]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.770655]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.771071]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.771472]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.771902]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> [   58.773337] Freed by task 23683:
> ...
> [   58.774815]  kfree+0xda/0x2b0
> [   58.775038]  free_reloc_control+0x1d6/0x220 [btrfs]
> [   58.775465]  btrfs_relocate_block_group+0x115c/0x1e20 [btrfs]
> [   58.775944]  btrfs_relocate_chunk+0xf1/0x760 [btrfs]
> [   58.776369]  __btrfs_balance+0x1326/0x1f10 [btrfs]
> [   58.776784]  btrfs_balance+0x3150/0x3d30 [btrfs]
> [   58.777185]  btrfs_ioctl_balance+0xd84/0x1410 [btrfs]
> [   58.777621]  btrfs_ioctl+0x4caa/0x114e0 [btrfs]
> ...
> 
> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>
> Signed-off-by: Zixuan Fu <r33s3n6@gmail.com>

Added to misc-next, thanks.