| Message ID | 20220721074829.2905233-1-r33s3n6@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: btrfs: fix a possible use-after-free bug caused by incorrect error handling in prepare_to_relocate() | expand |
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)
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>
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.
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)
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(-)