Skip to content

Commit a401c28

Browse files
YuKuai-huaweismb49
authored andcommitted
block, bfq: fix bfqq uaf in bfq_limit_depth()
Set new allocated bfqq to bic or remove freed bfqq from bic are both protected by bfqd->lock, however bfq_limit_depth() is deferencing bfqq from bic without the lock, this can lead to UAF if the io_context is shared by multiple tasks. For example, test bfq with io_uring can trigger following UAF in v6.6: ================================================================== BUG: KASAN: slab-use-after-free in bfqq_group+0x15/0x50 Call Trace: <TASK> dump_stack_lvl+0x47/0x80 print_address_description.constprop.0+0x66/0x300 print_report+0x3e/0x70 kasan_report+0xb4/0xf0 bfqq_group+0x15/0x50 bfqq_request_over_limit+0x130/0x9a0 bfq_limit_depth+0x1b5/0x480 __blk_mq_alloc_requests+0x2b5/0xa00 blk_mq_get_new_requests+0x11d/0x1d0 blk_mq_submit_bio+0x286/0xb00 submit_bio_noacct_nocheck+0x331/0x400 __block_write_full_folio+0x3d0/0x640 writepage_cb+0x3b/0xc0 write_cache_pages+0x254/0x6c0 write_cache_pages+0x254/0x6c0 do_writepages+0x192/0x310 filemap_fdatawrite_wbc+0x95/0xc0 __filemap_fdatawrite_range+0x99/0xd0 filemap_write_and_wait_range.part.0+0x4d/0xa0 blkdev_read_iter+0xef/0x1e0 io_read+0x1b6/0x8a0 io_issue_sqe+0x87/0x300 io_wq_submit_work+0xeb/0x390 io_worker_handle_work+0x24d/0x550 io_wq_worker+0x27f/0x6c0 ret_from_fork_asm+0x1b/0x30 </TASK> Allocated by task 808602: kasan_save_stack+0x1e/0x40 kasan_set_track+0x21/0x30 __kasan_slab_alloc+0x83/0x90 kmem_cache_alloc_node+0x1b1/0x6d0 bfq_get_queue+0x138/0xfa0 bfq_get_bfqq_handle_split+0xe3/0x2c0 bfq_init_rq+0x196/0xbb0 bfq_insert_request.isra.0+0xb5/0x480 bfq_insert_requests+0x156/0x180 blk_mq_insert_request+0x15d/0x440 blk_mq_submit_bio+0x8a4/0xb00 submit_bio_noacct_nocheck+0x331/0x400 __blkdev_direct_IO_async+0x2dd/0x330 blkdev_write_iter+0x39a/0x450 io_write+0x22a/0x840 io_issue_sqe+0x87/0x300 io_wq_submit_work+0xeb/0x390 io_worker_handle_work+0x24d/0x550 io_wq_worker+0x27f/0x6c0 ret_from_fork+0x2d/0x50 ret_from_fork_asm+0x1b/0x30 Freed by task 808589: kasan_save_stack+0x1e/0x40 kasan_set_track+0x21/0x30 kasan_save_free_info+0x27/0x40 __kasan_slab_free+0x126/0x1b0 kmem_cache_free+0x10c/0x750 bfq_put_queue+0x2dd/0x770 __bfq_insert_request.isra.0+0x155/0x7a0 bfq_insert_request.isra.0+0x122/0x480 bfq_insert_requests+0x156/0x180 blk_mq_dispatch_plug_list+0x528/0x7e0 blk_mq_flush_plug_list.part.0+0xe5/0x590 __blk_flush_plug+0x3b/0x90 blk_finish_plug+0x40/0x60 do_writepages+0x19d/0x310 filemap_fdatawrite_wbc+0x95/0xc0 __filemap_fdatawrite_range+0x99/0xd0 filemap_write_and_wait_range.part.0+0x4d/0xa0 blkdev_read_iter+0xef/0x1e0 io_read+0x1b6/0x8a0 io_issue_sqe+0x87/0x300 io_wq_submit_work+0xeb/0x390 io_worker_handle_work+0x24d/0x550 io_wq_worker+0x27f/0x6c0 ret_from_fork+0x2d/0x50 ret_from_fork_asm+0x1b/0x30 Fix the problem by protecting bic_to_bfqq() with bfqd->lock. CC: Jan Kara <jack@suse.cz> Fixes: 76f1df8 ("bfq: Limit number of requests consumed by each cgroup") Signed-off-by: Yu Kuai <yukuai3@huawei.com> Link: https://lore.kernel.org/r/20241129091509.2227136-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe <axboe@kernel.dk> CVE-2024-53166 (cherry picked from commit e8b8344de3980709080d86c157d24e7de07d70ad) Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Acked-by: Koichiro Den <koichiro.den@canonical.com> Acked-by: Thibault Ferrante <thibault.ferrante@canonical.com> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
1 parent 9b26ea1 commit a401c28

File tree

1 file changed

+24
-13
lines changed

1 file changed

+24
-13
lines changed

block/bfq-iosched.c

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -582,23 +582,31 @@ static struct request *bfq_choose_req(struct bfq_data *bfqd,
582582
#define BFQ_LIMIT_INLINE_DEPTH 16
583583

584584
#ifdef CONFIG_BFQ_GROUP_IOSCHED
585-
static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
585+
static bool bfqq_request_over_limit(struct bfq_data *bfqd,
586+
struct bfq_io_cq *bic, blk_opf_t opf,
587+
unsigned int act_idx, int limit)
586588
{
587-
struct bfq_data *bfqd = bfqq->bfqd;
588-
struct bfq_entity *entity = &bfqq->entity;
589589
struct bfq_entity *inline_entities[BFQ_LIMIT_INLINE_DEPTH];
590590
struct bfq_entity **entities = inline_entities;
591-
int depth, level, alloc_depth = BFQ_LIMIT_INLINE_DEPTH;
592-
int class_idx = bfqq->ioprio_class - 1;
591+
int alloc_depth = BFQ_LIMIT_INLINE_DEPTH;
593592
struct bfq_sched_data *sched_data;
593+
struct bfq_entity *entity;
594+
struct bfq_queue *bfqq;
594595
unsigned long wsum;
595596
bool ret = false;
596-
597-
if (!entity->on_st_or_in_serv)
598-
return false;
597+
int depth;
598+
int level;
599599

600600
retry:
601601
spin_lock_irq(&bfqd->lock);
602+
bfqq = bic_to_bfqq(bic, op_is_sync(opf), act_idx);
603+
if (!bfqq)
604+
goto out;
605+
606+
entity = &bfqq->entity;
607+
if (!entity->on_st_or_in_serv)
608+
goto out;
609+
602610
/* +1 for bfqq entity, root cgroup not included */
603611
depth = bfqg_to_blkg(bfqq_group(bfqq))->blkcg->css.cgroup->level + 1;
604612
if (depth > alloc_depth) {
@@ -643,7 +651,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
643651
* class.
644652
*/
645653
wsum = 0;
646-
for (i = 0; i <= class_idx; i++) {
654+
for (i = 0; i <= bfqq->ioprio_class - 1; i++) {
647655
wsum = wsum * IOPRIO_BE_NR +
648656
sched_data->service_tree[i].wsum;
649657
}
@@ -666,7 +674,9 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
666674
return ret;
667675
}
668676
#else
669-
static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
677+
static bool bfqq_request_over_limit(struct bfq_data *bfqd,
678+
struct bfq_io_cq *bic, blk_opf_t opf,
679+
unsigned int act_idx, int limit)
670680
{
671681
return false;
672682
}
@@ -704,16 +714,17 @@ static void bfq_limit_depth(blk_opf_t opf, struct blk_mq_alloc_data *data)
704714
}
705715

706716
for (act_idx = 0; bic && act_idx < bfqd->num_actuators; act_idx++) {
707-
struct bfq_queue *bfqq =
708-
bic_to_bfqq(bic, op_is_sync(opf), act_idx);
717+
/* Fast path to check if bfqq is already allocated. */
718+
if (!bic_to_bfqq(bic, op_is_sync(opf), act_idx))
719+
continue;
709720

710721
/*
711722
* Does queue (or any parent entity) exceed number of
712723
* requests that should be available to it? Heavily
713724
* limit depth so that it cannot consume more
714725
* available requests and thus starve other entities.
715726
*/
716-
if (bfqq && bfqq_request_over_limit(bfqq, limit)) {
727+
if (bfqq_request_over_limit(bfqd, bic, opf, act_idx, limit)) {
717728
depth = 1;
718729
break;
719730
}

0 commit comments

Comments
 (0)