| Message ID | 20250108084148.1549973-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block, bfq: fix waker_bfqq UAF after bfq_split_bfqq() | expand |
On Wed, 08 Jan 2025 16:41:48 +0800, Yu Kuai wrote: > Our syzkaller report a following UAF for v6.6: > > BUG: KASAN: slab-use-after-free in bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 > Read of size 8 at addr ffff8881b57147d8 by task fsstress/232726 > > CPU: 2 PID: 232726 Comm: fsstress Not tainted 6.6.0-g3629d1885222 #39 > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x91/0xf0 lib/dump_stack.c:106 > print_address_description.constprop.0+0x66/0x300 mm/kasan/report.c:364 > print_report+0x3e/0x70 mm/kasan/report.c:475 > kasan_report+0xb8/0xf0 mm/kasan/report.c:588 > hlist_add_head include/linux/list.h:1023 [inline] > bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 > bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 > bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 > blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 > blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 > __submit_bio+0xa0/0x6b0 block/blk-core.c:639 > __submit_bio_noacct_mq block/blk-core.c:718 [inline] > submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 > submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 > __ext4_read_bh fs/ext4/super.c:205 [inline] > ext4_read_bh+0x15e/0x2e0 fs/ext4/super.c:230 > __read_extent_tree_block+0x304/0x6f0 fs/ext4/extents.c:567 > ext4_find_extent+0x479/0xd20 fs/ext4/extents.c:947 > ext4_ext_map_blocks+0x1a3/0x2680 fs/ext4/extents.c:4182 > ext4_map_blocks+0x929/0x15a0 fs/ext4/inode.c:660 > ext4_iomap_begin_report+0x298/0x480 fs/ext4/inode.c:3569 > iomap_iter+0x3dd/0x1010 fs/iomap/iter.c:91 > iomap_fiemap+0x1f4/0x360 fs/iomap/fiemap.c:80 > ext4_fiemap+0x181/0x210 fs/ext4/extents.c:5051 > ioctl_fiemap.isra.0+0x1b4/0x290 fs/ioctl.c:220 > do_vfs_ioctl+0x31c/0x11a0 fs/ioctl.c:811 > __do_sys_ioctl fs/ioctl.c:869 [inline] > __se_sys_ioctl+0xae/0x190 fs/ioctl.c:857 > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > [...] Applied, thanks! [1/1] block, bfq: fix waker_bfqq UAF after bfq_split_bfqq() commit: eb92f7314625807ad569c218039ec90e9e14c784 Best regards,
On Wed 08-01-25 16:41:48, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > Our syzkaller report a following UAF for v6.6: > > BUG: KASAN: slab-use-after-free in bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 > Read of size 8 at addr ffff8881b57147d8 by task fsstress/232726 > > CPU: 2 PID: 232726 Comm: fsstress Not tainted 6.6.0-g3629d1885222 #39 > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x91/0xf0 lib/dump_stack.c:106 > print_address_description.constprop.0+0x66/0x300 mm/kasan/report.c:364 > print_report+0x3e/0x70 mm/kasan/report.c:475 > kasan_report+0xb8/0xf0 mm/kasan/report.c:588 > hlist_add_head include/linux/list.h:1023 [inline] > bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 > bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 > bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 > blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 > blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 > __submit_bio+0xa0/0x6b0 block/blk-core.c:639 > __submit_bio_noacct_mq block/blk-core.c:718 [inline] > submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 > submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 > __ext4_read_bh fs/ext4/super.c:205 [inline] > ext4_read_bh+0x15e/0x2e0 fs/ext4/super.c:230 > __read_extent_tree_block+0x304/0x6f0 fs/ext4/extents.c:567 > ext4_find_extent+0x479/0xd20 fs/ext4/extents.c:947 > ext4_ext_map_blocks+0x1a3/0x2680 fs/ext4/extents.c:4182 > ext4_map_blocks+0x929/0x15a0 fs/ext4/inode.c:660 > ext4_iomap_begin_report+0x298/0x480 fs/ext4/inode.c:3569 > iomap_iter+0x3dd/0x1010 fs/iomap/iter.c:91 > iomap_fiemap+0x1f4/0x360 fs/iomap/fiemap.c:80 > ext4_fiemap+0x181/0x210 fs/ext4/extents.c:5051 > ioctl_fiemap.isra.0+0x1b4/0x290 fs/ioctl.c:220 > do_vfs_ioctl+0x31c/0x11a0 fs/ioctl.c:811 > __do_sys_ioctl fs/ioctl.c:869 [inline] > __se_sys_ioctl+0xae/0x190 fs/ioctl.c:857 > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > Allocated by task 232719: > kasan_save_stack+0x22/0x50 mm/kasan/common.c:45 > kasan_set_track+0x25/0x30 mm/kasan/common.c:52 > __kasan_slab_alloc+0x87/0x90 mm/kasan/common.c:328 > kasan_slab_alloc include/linux/kasan.h:188 [inline] > slab_post_alloc_hook mm/slab.h:768 [inline] > slab_alloc_node mm/slub.c:3492 [inline] > kmem_cache_alloc_node+0x1b8/0x6f0 mm/slub.c:3537 > bfq_get_queue+0x215/0x1f00 block/bfq-iosched.c:5869 > bfq_get_bfqq_handle_split+0x167/0x5f0 block/bfq-iosched.c:6776 > bfq_init_rq+0x13a4/0x17a0 block/bfq-iosched.c:6938 > bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 > bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 > blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 > blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 > __submit_bio+0xa0/0x6b0 block/blk-core.c:639 > __submit_bio_noacct_mq block/blk-core.c:718 [inline] > submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 > submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 > __ext4_read_bh fs/ext4/super.c:205 [inline] > ext4_read_bh_nowait+0x15a/0x240 fs/ext4/super.c:217 > ext4_read_bh_lock+0xac/0xd0 fs/ext4/super.c:242 > ext4_bread_batch+0x268/0x500 fs/ext4/inode.c:958 > __ext4_find_entry+0x448/0x10f0 fs/ext4/namei.c:1671 > ext4_lookup_entry fs/ext4/namei.c:1774 [inline] > ext4_lookup.part.0+0x359/0x6f0 fs/ext4/namei.c:1842 > ext4_lookup+0x72/0x90 fs/ext4/namei.c:1839 > __lookup_slow+0x257/0x480 fs/namei.c:1696 > lookup_slow fs/namei.c:1713 [inline] > walk_component+0x454/0x5c0 fs/namei.c:2004 > link_path_walk.part.0+0x773/0xda0 fs/namei.c:2331 > link_path_walk fs/namei.c:3826 [inline] > path_openat+0x1b9/0x520 fs/namei.c:3826 > do_filp_open+0x1b7/0x400 fs/namei.c:3857 > do_sys_openat2+0x5dc/0x6e0 fs/open.c:1428 > do_sys_open fs/open.c:1443 [inline] > __do_sys_openat fs/open.c:1459 [inline] > __se_sys_openat fs/open.c:1454 [inline] > __x64_sys_openat+0x148/0x200 fs/open.c:1454 > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > Freed by task 232726: > kasan_save_stack+0x22/0x50 mm/kasan/common.c:45 > kasan_set_track+0x25/0x30 mm/kasan/common.c:52 > kasan_save_free_info+0x2b/0x50 mm/kasan/generic.c:522 > ____kasan_slab_free mm/kasan/common.c:236 [inline] > __kasan_slab_free+0x12a/0x1b0 mm/kasan/common.c:244 > kasan_slab_free include/linux/kasan.h:164 [inline] > slab_free_hook mm/slub.c:1827 [inline] > slab_free_freelist_hook mm/slub.c:1853 [inline] > slab_free mm/slub.c:3820 [inline] > kmem_cache_free+0x110/0x760 mm/slub.c:3842 > bfq_put_queue+0x6a7/0xfb0 block/bfq-iosched.c:5428 > bfq_forget_entity block/bfq-wf2q.c:634 [inline] > bfq_put_idle_entity+0x142/0x240 block/bfq-wf2q.c:645 > bfq_forget_idle+0x189/0x1e0 block/bfq-wf2q.c:671 > bfq_update_vtime block/bfq-wf2q.c:1280 [inline] > __bfq_lookup_next_entity block/bfq-wf2q.c:1374 [inline] > bfq_lookup_next_entity+0x350/0x480 block/bfq-wf2q.c:1433 > bfq_update_next_in_service+0x1c0/0x4f0 block/bfq-wf2q.c:128 > bfq_deactivate_entity+0x10a/0x240 block/bfq-wf2q.c:1188 > bfq_deactivate_bfqq block/bfq-wf2q.c:1592 [inline] > bfq_del_bfqq_busy+0x2e8/0xad0 block/bfq-wf2q.c:1659 > bfq_release_process_ref+0x1cc/0x220 block/bfq-iosched.c:3139 > bfq_split_bfqq+0x481/0xdf0 block/bfq-iosched.c:6754 > bfq_init_rq+0xf29/0x17a0 block/bfq-iosched.c:6934 > bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 > bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 > blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 > blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 > __submit_bio+0xa0/0x6b0 block/blk-core.c:639 > __submit_bio_noacct_mq block/blk-core.c:718 [inline] > submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 > submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 > __ext4_read_bh fs/ext4/super.c:205 [inline] > ext4_read_bh+0x15e/0x2e0 fs/ext4/super.c:230 > __read_extent_tree_block+0x304/0x6f0 fs/ext4/extents.c:567 > ext4_find_extent+0x479/0xd20 fs/ext4/extents.c:947 > ext4_ext_map_blocks+0x1a3/0x2680 fs/ext4/extents.c:4182 > ext4_map_blocks+0x929/0x15a0 fs/ext4/inode.c:660 > ext4_iomap_begin_report+0x298/0x480 fs/ext4/inode.c:3569 > iomap_iter+0x3dd/0x1010 fs/iomap/iter.c:91 > iomap_fiemap+0x1f4/0x360 fs/iomap/fiemap.c:80 > ext4_fiemap+0x181/0x210 fs/ext4/extents.c:5051 > ioctl_fiemap.isra.0+0x1b4/0x290 fs/ioctl.c:220 > do_vfs_ioctl+0x31c/0x11a0 fs/ioctl.c:811 > __do_sys_ioctl fs/ioctl.c:869 [inline] > __se_sys_ioctl+0xae/0x190 fs/ioctl.c:857 > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > commit 1ba0403ac644 ("block, bfq: fix uaf for accessing waker_bfqq after > splitting") fix the problem that if waker_bfqq is in the merge chain, > and current is the only procress, waker_bfqq can be freed from > bfq_split_bfqq(). However, the case that waker_bfqq is not in the merge > chain is missed, and if the procress reference of waker_bfqq is 0, > waker_bfqq can be freed as well. > > Fix the problem by checking procress reference if waker_bfqq is not in > the merge_chain. > > Fixes: 1ba0403ac644 ("block, bfq: fix uaf for accessing waker_bfqq after splitting") > Signed-off-by: Hou Tao <houtao1@huawei.com> > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > block/bfq-iosched.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > index 068c63e95738..c69e9faad15a 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -6844,16 +6844,24 @@ static struct bfq_queue *bfq_waker_bfqq(struct bfq_queue *bfqq) > if (new_bfqq == waker_bfqq) { > /* > * If waker_bfqq is in the merge chain, and current > - * is the only procress. > + * is the only procress, waker_bfqq can be freed. ^^^ can you please fix the typo while modifying the comment? It should be "process". > */ > if (bfqq_process_refs(waker_bfqq) == 1) > return NULL; > - break; > + > + return waker_bfqq; So how do you know bfqq_process_refs(waker_bfqq) is not 0 in this case? > } > > new_bfqq = new_bfqq->new_bfqq; > } > > + /* > + * If waker_bfqq is not in the merge chain, and it's procress reference ^^^ process > + * is 0, waker_bfqq can be freed. > + */ > + if (bfqq_process_refs(waker_bfqq) == 0) > + return NULL; > + > return waker_bfqq; > } Honza
Hi, 在 2025/01/08 22:42, Jan Kara 写道: > On Wed 08-01-25 16:41:48, Yu Kuai wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> Our syzkaller report a following UAF for v6.6: >> >> BUG: KASAN: slab-use-after-free in bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 >> Read of size 8 at addr ffff8881b57147d8 by task fsstress/232726 >> >> CPU: 2 PID: 232726 Comm: fsstress Not tainted 6.6.0-g3629d1885222 #39 >> Call Trace: >> <TASK> >> __dump_stack lib/dump_stack.c:88 [inline] >> dump_stack_lvl+0x91/0xf0 lib/dump_stack.c:106 >> print_address_description.constprop.0+0x66/0x300 mm/kasan/report.c:364 >> print_report+0x3e/0x70 mm/kasan/report.c:475 >> kasan_report+0xb8/0xf0 mm/kasan/report.c:588 >> hlist_add_head include/linux/list.h:1023 [inline] >> bfq_init_rq+0x175d/0x17a0 block/bfq-iosched.c:6958 >> bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 >> bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 >> blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 >> blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 >> __submit_bio+0xa0/0x6b0 block/blk-core.c:639 >> __submit_bio_noacct_mq block/blk-core.c:718 [inline] >> submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 >> submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 >> __ext4_read_bh fs/ext4/super.c:205 [inline] >> ext4_read_bh+0x15e/0x2e0 fs/ext4/super.c:230 >> __read_extent_tree_block+0x304/0x6f0 fs/ext4/extents.c:567 >> ext4_find_extent+0x479/0xd20 fs/ext4/extents.c:947 >> ext4_ext_map_blocks+0x1a3/0x2680 fs/ext4/extents.c:4182 >> ext4_map_blocks+0x929/0x15a0 fs/ext4/inode.c:660 >> ext4_iomap_begin_report+0x298/0x480 fs/ext4/inode.c:3569 >> iomap_iter+0x3dd/0x1010 fs/iomap/iter.c:91 >> iomap_fiemap+0x1f4/0x360 fs/iomap/fiemap.c:80 >> ext4_fiemap+0x181/0x210 fs/ext4/extents.c:5051 >> ioctl_fiemap.isra.0+0x1b4/0x290 fs/ioctl.c:220 >> do_vfs_ioctl+0x31c/0x11a0 fs/ioctl.c:811 >> __do_sys_ioctl fs/ioctl.c:869 [inline] >> __se_sys_ioctl+0xae/0x190 fs/ioctl.c:857 >> do_syscall_x64 arch/x86/entry/common.c:51 [inline] >> do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 >> entry_SYSCALL_64_after_hwframe+0x78/0xe2 >> >> Allocated by task 232719: >> kasan_save_stack+0x22/0x50 mm/kasan/common.c:45 >> kasan_set_track+0x25/0x30 mm/kasan/common.c:52 >> __kasan_slab_alloc+0x87/0x90 mm/kasan/common.c:328 >> kasan_slab_alloc include/linux/kasan.h:188 [inline] >> slab_post_alloc_hook mm/slab.h:768 [inline] >> slab_alloc_node mm/slub.c:3492 [inline] >> kmem_cache_alloc_node+0x1b8/0x6f0 mm/slub.c:3537 >> bfq_get_queue+0x215/0x1f00 block/bfq-iosched.c:5869 >> bfq_get_bfqq_handle_split+0x167/0x5f0 block/bfq-iosched.c:6776 >> bfq_init_rq+0x13a4/0x17a0 block/bfq-iosched.c:6938 >> bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 >> bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 >> blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 >> blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 >> __submit_bio+0xa0/0x6b0 block/blk-core.c:639 >> __submit_bio_noacct_mq block/blk-core.c:718 [inline] >> submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 >> submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 >> __ext4_read_bh fs/ext4/super.c:205 [inline] >> ext4_read_bh_nowait+0x15a/0x240 fs/ext4/super.c:217 >> ext4_read_bh_lock+0xac/0xd0 fs/ext4/super.c:242 >> ext4_bread_batch+0x268/0x500 fs/ext4/inode.c:958 >> __ext4_find_entry+0x448/0x10f0 fs/ext4/namei.c:1671 >> ext4_lookup_entry fs/ext4/namei.c:1774 [inline] >> ext4_lookup.part.0+0x359/0x6f0 fs/ext4/namei.c:1842 >> ext4_lookup+0x72/0x90 fs/ext4/namei.c:1839 >> __lookup_slow+0x257/0x480 fs/namei.c:1696 >> lookup_slow fs/namei.c:1713 [inline] >> walk_component+0x454/0x5c0 fs/namei.c:2004 >> link_path_walk.part.0+0x773/0xda0 fs/namei.c:2331 >> link_path_walk fs/namei.c:3826 [inline] >> path_openat+0x1b9/0x520 fs/namei.c:3826 >> do_filp_open+0x1b7/0x400 fs/namei.c:3857 >> do_sys_openat2+0x5dc/0x6e0 fs/open.c:1428 >> do_sys_open fs/open.c:1443 [inline] >> __do_sys_openat fs/open.c:1459 [inline] >> __se_sys_openat fs/open.c:1454 [inline] >> __x64_sys_openat+0x148/0x200 fs/open.c:1454 >> do_syscall_x64 arch/x86/entry/common.c:51 [inline] >> do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 >> entry_SYSCALL_64_after_hwframe+0x78/0xe2 >> >> Freed by task 232726: >> kasan_save_stack+0x22/0x50 mm/kasan/common.c:45 >> kasan_set_track+0x25/0x30 mm/kasan/common.c:52 >> kasan_save_free_info+0x2b/0x50 mm/kasan/generic.c:522 >> ____kasan_slab_free mm/kasan/common.c:236 [inline] >> __kasan_slab_free+0x12a/0x1b0 mm/kasan/common.c:244 >> kasan_slab_free include/linux/kasan.h:164 [inline] >> slab_free_hook mm/slub.c:1827 [inline] >> slab_free_freelist_hook mm/slub.c:1853 [inline] >> slab_free mm/slub.c:3820 [inline] >> kmem_cache_free+0x110/0x760 mm/slub.c:3842 >> bfq_put_queue+0x6a7/0xfb0 block/bfq-iosched.c:5428 >> bfq_forget_entity block/bfq-wf2q.c:634 [inline] >> bfq_put_idle_entity+0x142/0x240 block/bfq-wf2q.c:645 >> bfq_forget_idle+0x189/0x1e0 block/bfq-wf2q.c:671 >> bfq_update_vtime block/bfq-wf2q.c:1280 [inline] >> __bfq_lookup_next_entity block/bfq-wf2q.c:1374 [inline] >> bfq_lookup_next_entity+0x350/0x480 block/bfq-wf2q.c:1433 >> bfq_update_next_in_service+0x1c0/0x4f0 block/bfq-wf2q.c:128 >> bfq_deactivate_entity+0x10a/0x240 block/bfq-wf2q.c:1188 >> bfq_deactivate_bfqq block/bfq-wf2q.c:1592 [inline] >> bfq_del_bfqq_busy+0x2e8/0xad0 block/bfq-wf2q.c:1659 >> bfq_release_process_ref+0x1cc/0x220 block/bfq-iosched.c:3139 >> bfq_split_bfqq+0x481/0xdf0 block/bfq-iosched.c:6754 >> bfq_init_rq+0xf29/0x17a0 block/bfq-iosched.c:6934 >> bfq_insert_request.isra.0+0xe8/0xa20 block/bfq-iosched.c:6271 >> bfq_insert_requests+0x27f/0x390 block/bfq-iosched.c:6323 >> blk_mq_insert_request+0x290/0x8f0 block/blk-mq.c:2660 >> blk_mq_submit_bio+0x1021/0x15e0 block/blk-mq.c:3143 >> __submit_bio+0xa0/0x6b0 block/blk-core.c:639 >> __submit_bio_noacct_mq block/blk-core.c:718 [inline] >> submit_bio_noacct_nocheck+0x5b7/0x810 block/blk-core.c:747 >> submit_bio_noacct+0xca0/0x1990 block/blk-core.c:847 >> __ext4_read_bh fs/ext4/super.c:205 [inline] >> ext4_read_bh+0x15e/0x2e0 fs/ext4/super.c:230 >> __read_extent_tree_block+0x304/0x6f0 fs/ext4/extents.c:567 >> ext4_find_extent+0x479/0xd20 fs/ext4/extents.c:947 >> ext4_ext_map_blocks+0x1a3/0x2680 fs/ext4/extents.c:4182 >> ext4_map_blocks+0x929/0x15a0 fs/ext4/inode.c:660 >> ext4_iomap_begin_report+0x298/0x480 fs/ext4/inode.c:3569 >> iomap_iter+0x3dd/0x1010 fs/iomap/iter.c:91 >> iomap_fiemap+0x1f4/0x360 fs/iomap/fiemap.c:80 >> ext4_fiemap+0x181/0x210 fs/ext4/extents.c:5051 >> ioctl_fiemap.isra.0+0x1b4/0x290 fs/ioctl.c:220 >> do_vfs_ioctl+0x31c/0x11a0 fs/ioctl.c:811 >> __do_sys_ioctl fs/ioctl.c:869 [inline] >> __se_sys_ioctl+0xae/0x190 fs/ioctl.c:857 >> do_syscall_x64 arch/x86/entry/common.c:51 [inline] >> do_syscall_64+0x70/0x120 arch/x86/entry/common.c:81 >> entry_SYSCALL_64_after_hwframe+0x78/0xe2 >> >> commit 1ba0403ac644 ("block, bfq: fix uaf for accessing waker_bfqq after >> splitting") fix the problem that if waker_bfqq is in the merge chain, >> and current is the only procress, waker_bfqq can be freed from >> bfq_split_bfqq(). However, the case that waker_bfqq is not in the merge >> chain is missed, and if the procress reference of waker_bfqq is 0, >> waker_bfqq can be freed as well. >> >> Fix the problem by checking procress reference if waker_bfqq is not in >> the merge_chain. >> >> Fixes: 1ba0403ac644 ("block, bfq: fix uaf for accessing waker_bfqq after splitting") >> Signed-off-by: Hou Tao <houtao1@huawei.com> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >> --- >> block/bfq-iosched.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c >> index 068c63e95738..c69e9faad15a 100644 >> --- a/block/bfq-iosched.c >> +++ b/block/bfq-iosched.c >> @@ -6844,16 +6844,24 @@ static struct bfq_queue *bfq_waker_bfqq(struct bfq_queue *bfqq) >> if (new_bfqq == waker_bfqq) { >> /* >> * If waker_bfqq is in the merge chain, and current >> - * is the only procress. >> + * is the only procress, waker_bfqq can be freed. > ^^^ can you please fix the typo > while modifying the comment? It should be "process". Sorry for the typo, I didn't notice it at all. :( > > >> */ >> if (bfqq_process_refs(waker_bfqq) == 1) >> return NULL; >> - break; >> + >> + return waker_bfqq; > > So how do you know bfqq_process_refs(waker_bfqq) is not 0 in this case? Because in this case, waker_bfqq is in the merge chain of bfqq, and bfqq is obtained frm the current process, which means waker_bfqq should have at least one process reference that is from current thread. Thanks, Kuai > >> } >> >> new_bfqq = new_bfqq->new_bfqq; >> } >> >> + /* >> + * If waker_bfqq is not in the merge chain, and it's procress reference > ^^^ process >> + * is 0, waker_bfqq can be freed. >> + */ >> + if (bfqq_process_refs(waker_bfqq) == 0) >> + return NULL; >> + >> return waker_bfqq; >> } > > Honza >
On Thu 09-01-25 09:32:08, Yu Kuai wrote: > Hi, > > 在 2025/01/08 22:42, Jan Kara 写道: > > > > > > > */ > > > if (bfqq_process_refs(waker_bfqq) == 1) > > > return NULL; > > > - break; > > > + > > > + return waker_bfqq; > > > > So how do you know bfqq_process_refs(waker_bfqq) is not 0 in this case? > > Because in this case, waker_bfqq is in the merge chain of bfqq, and bfqq > is obtained frm the current process, which means waker_bfqq should have > at least one process reference that is from current thread. Ah, right. Thanks for explanation. The except for the typo the patch looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> (although I can see Jens has already picked up the patch so probably this is immaterial). Honza
On 1/9/25 1:50 AM, Jan Kara wrote: > On Thu 09-01-25 09:32:08, Yu Kuai wrote: >> Hi, >> >> ? 2025/01/08 22:42, Jan Kara ??: >>> >>> >>>> */ >>>> if (bfqq_process_refs(waker_bfqq) == 1) >>>> return NULL; >>>> - break; >>>> + >>>> + return waker_bfqq; >>> >>> So how do you know bfqq_process_refs(waker_bfqq) is not 0 in this case? >> >> Because in this case, waker_bfqq is in the merge chain of bfqq, and bfqq >> is obtained frm the current process, which means waker_bfqq should have >> at least one process reference that is from current thread. > > Ah, right. Thanks for explanation. The except for the typo the patch looks > good to me. Feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > (although I can see Jens has already picked up the patch so probably this > is immaterial). Still useful! The patch has a link to this thread, so it's still connected even if the commit itself isn't updated. Though with the typo in process, I'm kind of pondering just amending the commit and then I'll add your reviewed-by as well. But usually I don't, but still appreciate reviews after it's been queued.
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 068c63e95738..c69e9faad15a 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -6844,16 +6844,24 @@ static struct bfq_queue *bfq_waker_bfqq(struct bfq_queue *bfqq) if (new_bfqq == waker_bfqq) { /* * If waker_bfqq is in the merge chain, and current - * is the only procress. + * is the only procress, waker_bfqq can be freed. */ if (bfqq_process_refs(waker_bfqq) == 1) return NULL; - break; + + return waker_bfqq; } new_bfqq = new_bfqq->new_bfqq; } + /* + * If waker_bfqq is not in the merge chain, and it's procress reference + * is 0, waker_bfqq can be freed. + */ + if (bfqq_process_refs(waker_bfqq) == 0) + return NULL; + return waker_bfqq; }