| Message ID | ecbec03d147e441bb5b91a0cae3fb782@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request | expand |
> Il giorno 23 dic 2021, alle ore 03:06, zhangwensheng (E) <zhangwensheng5@huawei.com> ha scritto: > > ping... > > -----邮件原件----- > 发件人: zhangwensheng (E) > 发送时间: 2021年12月20日 10:35 > 收件人: 'Paolo Valente' <paolo.valente@linaro.org> > 抄送: Jens Axboe <axboe@kernel.dk>; linux-block@vger.kernel.org; linux-kernel@vger.kernel.org > 主题: 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request > > Hi paolo： > Thanks for your question, it is my negligence. > I have two ideas for repairing the problem: > Hi, sorry for the delay. I've been thinking of this UAF a little bit more, and I've realized that in_serv_queue may become a pending pointer only if in_serv_queue has no rq queued. But, if in_serv_queue has no rq queued, then idle_timer_disabled will necessarily be false. And, if idle_timer_disabled is false, then the parameter in_serv_queue will not be used inside bfq_update_dispatch_stats. So, after your initial fix, the rest of the code seems already correct, with no change. Maybe, if we don't like to pass a pending pointer, what about something like ... bfq_update_dispatch_stats(hctx->queue, rq, idle_timer_disabled ? in_serv_queue : NULL, idle_timer_disabled); (the above is just to give express my idea, please write better code if needed). Thanks, Paolo > 1. use ref++ to avoid the in_serv_queue being released. Patch as follow: > > block/bfq-iosched.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index fec18118dc30..70bd280170f9 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -5066,6 +5066,7 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) > spin_lock_irq(&bfqd->lock); > > in_serv_queue = bfqd->in_service_queue; > + in_serv_queue->ref++; /* aviod in_serv_queue release */ > waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); > > rq = __bfq_dispatch_request(hctx); @@ -5077,6 +5078,10 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) > > bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, > idle_timer_disabled); > + /* resume in_serv_queue */ > + spin_lock_irq(&bfqd->lock); > + bfq_put_queue(in_serv_queue); > + spin_unlock_irq(&bfqd->lock); > > return rq; > } > > 2. add new changes to previous, taking out bfqq_group(in_serv_queue) from bfq_update_dispatch_stats. Patch as follow: > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -5007,7 +5007,7 @@ static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) #ifdef CONFIG_BFQ_CGROUP_DEBUG static void bfq_update_dispatch_stats(struct request_queue *q, > struct request *rq, > - struct bfq_queue *in_serv_queue, > + struct bfq_group *bfqg_in_serv, > bool idle_timer_disabled) { > struct bfq_queue *bfqq = rq ? RQ_BFQQ(rq) : NULL; @@ -5039,7 +5039,7 @@ static void bfq_update_dispatch_stats(struct request_queue *q, > * therefore guaranteed to exist because of the above > * arguments. > */ > - bfqg_stats_update_idle_time(bfqq_group(in_serv_queue)); > + bfqg_stats_update_idle_time(bfqg_in_serv); > if (bfqq) { > struct bfq_group *bfqg = bfqq_group(bfqq); > > @@ -5052,7 +5052,7 @@ static void bfq_update_dispatch_stats(struct request_queue *q, #else static inline void bfq_update_dispatch_stats(struct request_queue *q, > struct request *rq, > - struct bfq_queue *in_serv_queue, > + struct bfq_group > + *bfqg_in_serv, > bool idle_timer_disabled) {} #endif /* CONFIG_BFQ_CGROUP_DEBUG */ > > @@ -5062,20 +5062,23 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) > struct request *rq; > struct bfq_queue *in_serv_queue; > bool waiting_rq, idle_timer_disabled; > + struct bfq_group *bfqg_in_serv; > > spin_lock_irq(&bfqd->lock); > > in_serv_queue = bfqd->in_service_queue; > waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); > + bfqg_in_serv = bfqq_group(in_serv_queue); > > rq = __bfq_dispatch_request(hctx); > > - idle_timer_disabled = > - waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); > - > + if (in_serv_queue == bfqd->in_service_queue) { > + idle_timer_disabled = > + waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); > + } > spin_unlock_irq(&bfqd->lock); > > - bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, > + bfq_update_dispatch_stats(hctx->queue, rq, bfqg_in_serv, > idle_timer_disabled); > > return rq; > > what do you think? > > Thanks > Zhang Wensheng > -----邮件原件----- > 发件人: Paolo Valente [mailto:paolo.valente@linaro.org] > 发送时间: 2021年12月17日 0:28 > 收件人: zhangwensheng (E) <zhangwensheng5@huawei.com> > 抄送: Jens Axboe <axboe@kernel.dk>; linux-block@vger.kernel.org; linux-kernel@vger.kernel.org > 主题: Re: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request > > > >> Il giorno 16 dic 2021, alle ore 13:21, Zhang Wensheng <zhangwensheng5@huawei.com> ha scritto: >> >> KASAN reports a use-after-free report when doing normal scsi-mq test >> >> [69832.239032] >> ================================================================== >> [69832.241810] BUG: KASAN: use-after-free in >> bfq_dispatch_request+0x1045/0x44b0 >> [69832.243267] Read of size 8 at addr ffff88802622ba88 by task >> kworker/3:1H/155 [69832.244656] [69832.245007] CPU: 3 PID: 155 Comm: >> kworker/3:1H Not tainted 5.10.0-10295-g576c6382529e #8 [69832.246626] >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >> rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014 [69832.249069] >> Workqueue: kblockd blk_mq_run_work_fn [69832.250022] Call Trace: >> [69832.250541] dump_stack+0x9b/0xce >> [69832.251232] ? bfq_dispatch_request+0x1045/0x44b0 >> [69832.252243] print_address_description.constprop.6+0x3e/0x60 >> [69832.253381] ? __cpuidle_text_end+0x5/0x5 [69832.254211] ? >> vprintk_func+0x6b/0x120 [69832.254994] ? >> bfq_dispatch_request+0x1045/0x44b0 >> [69832.255952] ? bfq_dispatch_request+0x1045/0x44b0 >> [69832.256914] kasan_report.cold.9+0x22/0x3a [69832.257753] ? >> bfq_dispatch_request+0x1045/0x44b0 >> [69832.258755] check_memory_region+0x1c1/0x1e0 [69832.260248] >> bfq_dispatch_request+0x1045/0x44b0 >> [69832.261181] ? bfq_bfqq_expire+0x2440/0x2440 [69832.262032] ? >> blk_mq_delay_run_hw_queues+0xf9/0x170 >> [69832.263022] __blk_mq_do_dispatch_sched+0x52f/0x830 >> [69832.264011] ? blk_mq_sched_request_inserted+0x100/0x100 >> [69832.265101] __blk_mq_sched_dispatch_requests+0x398/0x4f0 >> [69832.266206] ? blk_mq_do_dispatch_ctx+0x570/0x570 >> [69832.267147] ? __switch_to+0x5f4/0xee0 [69832.267898] >> blk_mq_sched_dispatch_requests+0xdf/0x140 >> [69832.268946] __blk_mq_run_hw_queue+0xc0/0x270 [69832.269840] >> blk_mq_run_work_fn+0x51/0x60 [69832.278170] >> process_one_work+0x6d4/0xfe0 [69832.278984] worker_thread+0x91/0xc80 >> [69832.279726] ? __kthread_parkme+0xb0/0x110 [69832.280554] ? >> process_one_work+0xfe0/0xfe0 [69832.281414] kthread+0x32d/0x3f0 >> [69832.282082] ? kthread_park+0x170/0x170 [69832.282849] >> ret_from_fork+0x1f/0x30 [69832.283573] [69832.283886] Allocated by >> task 7725: >> [69832.284599] kasan_save_stack+0x19/0x40 [69832.285385] >> __kasan_kmalloc.constprop.2+0xc1/0xd0 >> [69832.286350] kmem_cache_alloc_node+0x13f/0x460 [69832.287237] >> bfq_get_queue+0x3d4/0x1140 [69832.287993] >> bfq_get_bfqq_handle_split+0x103/0x510 >> [69832.289015] bfq_init_rq+0x337/0x2d50 [69832.289749] >> bfq_insert_requests+0x304/0x4e10 [69832.290634] >> blk_mq_sched_insert_requests+0x13e/0x390 >> [69832.291629] blk_mq_flush_plug_list+0x4b4/0x760 >> [69832.292538] blk_flush_plug_list+0x2c5/0x480 [69832.293392] >> io_schedule_prepare+0xb2/0xd0 [69832.294209] >> io_schedule_timeout+0x13/0x80 [69832.295014] >> wait_for_common_io.constprop.1+0x13c/0x270 >> [69832.296137] submit_bio_wait+0x103/0x1a0 [69832.296932] >> blkdev_issue_discard+0xe6/0x160 [69832.297794] >> blk_ioctl_discard+0x219/0x290 [69832.298614] >> blkdev_common_ioctl+0x50a/0x1750 [69832.304715] >> blkdev_ioctl+0x470/0x600 [69832.305474] block_ioctl+0xde/0x120 >> [69832.306232] vfs_ioctl+0x6c/0xc0 [69832.306877] >> __se_sys_ioctl+0x90/0xa0 [69832.307629] do_syscall_64+0x2d/0x40 >> [69832.308362] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> [69832.309382] >> [69832.309701] Freed by task 155: >> [69832.310328] kasan_save_stack+0x19/0x40 [69832.311121] >> kasan_set_track+0x1c/0x30 [69832.311868] >> kasan_set_free_info+0x1b/0x30 [69832.312699] >> __kasan_slab_free+0x111/0x160 [69832.313524] >> kmem_cache_free+0x94/0x460 [69832.314367] bfq_put_queue+0x582/0x940 >> [69832.315112] __bfq_bfqd_reset_in_service+0x166/0x1d0 >> [69832.317275] bfq_bfqq_expire+0xb27/0x2440 [69832.318084] >> bfq_dispatch_request+0x697/0x44b0 [69832.318991] >> __blk_mq_do_dispatch_sched+0x52f/0x830 >> [69832.319984] __blk_mq_sched_dispatch_requests+0x398/0x4f0 >> [69832.321087] blk_mq_sched_dispatch_requests+0xdf/0x140 >> [69832.322225] __blk_mq_run_hw_queue+0xc0/0x270 [69832.323114] >> blk_mq_run_work_fn+0x51/0x60 [69832.323942] >> process_one_work+0x6d4/0xfe0 [69832.324772] worker_thread+0x91/0xc80 >> [69832.325518] kthread+0x32d/0x3f0 [69832.326205] >> ret_from_fork+0x1f/0x30 [69832.326932] [69832.338297] The buggy >> address belongs to the object at ffff88802622b968 [69832.338297] >> which belongs to the cache bfq_queue of size 512 [69832.340766] The >> buggy address is located 288 bytes inside of [69832.340766] 512-byte >> region [ffff88802622b968, ffff88802622bb68) [69832.343091] The buggy >> address belongs to the page: >> [69832.344097] page:ffffea0000988a00 refcount:1 mapcount:0 >> mapping:0000000000000000 index:0xffff88802622a528 pfn:0x26228 >> [69832.346214] head:ffffea0000988a00 order:2 compound_mapcount:0 >> compound_pincount:0 [69832.347719] flags: 0x1fffff80010200(slab|head) >> [69832.348625] raw: 001fffff80010200 ffffea0000dbac08 ffff888017a57650 >> ffff8880179fe840 [69832.354972] raw: ffff88802622a528 0000000000120008 >> 00000001ffffffff 0000000000000000 [69832.356547] page dumped because: >> kasan: bad access detected [69832.357652] [69832.357970] Memory state around the buggy address: >> [69832.358926] ffff88802622b980: fb fb fb fb fb fb fb fb fb fb fb fb >> fb fb fb fb [69832.360358] ffff88802622ba00: fb fb fb fb fb fb fb fb >> fb fb fb fb fb fb fb fb [69832.361810] >ffff88802622ba80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >> [69832.363273] ^ >> [69832.363975] ffff88802622bb00: fb fb fb fb fb fb fb fb fb fb fb fb >> fb fc fc fc [69832.375960] ffff88802622bb80: fc fc fc fc fc fc fc fc >> fc fc fc fc fc fc fc fc [69832.377405] >> ================================================================== >> >> In bfq_dispatch_requestfunction, it may have function call: >> >> bfq_dispatch_request >> __bfq_dispatch_request >> bfq_select_queue >> bfq_bfqq_expire >> __bfq_bfqd_reset_in_service >> bfq_put_queue >> kmem_cache_free >> In this function call, in_serv_queue has beed expired and meet the >> conditions to free. In the function bfq_dispatch_request, the address >> of in_serv_queue pointing to has been released. For getting the value >> of idle_timer_disabled, it will get flags value from the address which >> in_serv_queue pointing to, then the problem of use-after-free happens; >> >> Fix the problem by check in_serv_queue == bfqd->in_service_queue, to >> get the value of idle_timer_disabled if in_serve_queue is equel to >> bfqd->in_service_queue. If the space of in_serv_queue pointing has >> been released, this judge will aviod use-after-free problem. >> And if in_serv_queue may be expired but it still exists, this judge >> may have little effects on the function bfqg_stats_update_idle_time in >> bfq_update_dispatch_stats. >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Zhang Wensheng <zhangwensheng5@huawei.com> >> --- >> block/bfq-iosched.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index >> fec18118dc30..97533634b99e 100644 >> --- a/block/bfq-iosched.c >> +++ b/block/bfq-iosched.c >> @@ -5061,7 +5061,7 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) >> struct bfq_data *bfqd = hctx->queue->elevator->elevator_data; >> struct request *rq; >> struct bfq_queue *in_serv_queue; >> - bool waiting_rq, idle_timer_disabled; >> + bool waiting_rq, idle_timer_disabled = false; >> >> spin_lock_irq(&bfqd->lock); >> >> @@ -5070,9 +5070,10 @@ static struct request >> *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) >> >> rq = __bfq_dispatch_request(hctx); >> >> - idle_timer_disabled = >> - waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); >> - >> + if (in_serv_queue == bfqd->in_service_queue) { >> + idle_timer_disabled = >> + waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); >> + } > > Good catch! > >> spin_unlock_irq(&bfqd->lock); >> >> bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, > > Yet, what about the above use of in_serv_queue then? > > Thanks, > Paolo > >> -- >> 2.31.1 >> >
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index fec18118dc30..70bd280170f9 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5066,6 +5066,7 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) spin_lock_irq(&bfqd->lock); in_serv_queue = bfqd->in_service_queue; + in_serv_queue->ref++; /* aviod in_serv_queue release */ waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); rq = __bfq_dispatch_request(hctx); @@ -5077,6 +5078,10 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, idle_timer_disabled); + /* resume in_serv_queue */ + spin_lock_irq(&bfqd->lock); + bfq_put_queue(in_serv_queue); + spin_unlock_irq(&bfqd->lock); return rq; } 2. add new changes to previous, taking out bfqq_group(in_serv_queue) from bfq_update_dispatch_stats. Patch as follow: diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5007,7 +5007,7 @@ static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) #ifdef CONFIG_BFQ_CGROUP_DEBUG static void bfq_update_dispatch_stats(struct request_queue *q, struct request *rq, - struct bfq_queue *in_serv_queue, + struct bfq_group *bfqg_in_serv, bool idle_timer_disabled) { struct bfq_queue *bfqq = rq ? RQ_BFQQ(rq) : NULL; @@ -5039,7 +5039,7 @@ static void bfq_update_dispatch_stats(struct request_queue *q, * therefore guaranteed to exist because of the above * arguments. */
ping... -----邮件原件----- 发件人: zhangwensheng (E) 发送时间: 2021年12月20日 10:35 收件人: 'Paolo Valente' <paolo.valente@linaro.org> 抄送: Jens Axboe <axboe@kernel.dk>; linux-block@vger.kernel.org; linux-kernel@vger.kernel.org 主题: 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request Hi paolo： Thanks for your question, it is my negligence. I have two ideas for repairing the problem: 1. use ref++ to avoid the in_serv_queue being released. Patch as follow: block/bfq-iosched.c | 5 +++++ 1 file changed, 5 insertions(+) - bfqg_stats_update_idle_time(bfqq_group(in_serv_queue)); + bfqg_stats_update_idle_time(bfqg_in_serv); if (bfqq) { struct bfq_group *bfqg = bfqq_group(bfqq); @@ -5052,7 +5052,7 @@ static void bfq_update_dispatch_stats(struct request_queue *q, #else static inline void bfq_update_dispatch_stats(struct request_queue *q, struct request *rq, - struct bfq_queue *in_serv_queue, + struct bfq_group + *bfqg_in_serv, bool idle_timer_disabled) {} #endif /* CONFIG_BFQ_CGROUP_DEBUG */ @@ -5062,20 +5062,23 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) struct request *rq; struct bfq_queue *in_serv_queue; bool waiting_rq, idle_timer_disabled; + struct bfq_group *bfqg_in_serv; spin_lock_irq(&bfqd->lock); in_serv_queue = bfqd->in_service_queue; waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); + bfqg_in_serv = bfqq_group(in_serv_queue); rq = __bfq_dispatch_request(hctx); - idle_timer_disabled = - waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); - + if (in_serv_queue == bfqd->in_service_queue) { + idle_timer_disabled = + waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); + } spin_unlock_irq(&bfqd->lock); - bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, + bfq_update_dispatch_stats(hctx->queue, rq, bfqg_in_serv, idle_timer_disabled); return rq; what do you think? Thanks Zhang Wensheng -----邮件原件----- 发件人: Paolo Valente [mailto:paolo.valente@linaro.org] 发送时间: 2021年12月17日 0:28 收件人: zhangwensheng (E) <zhangwensheng5@huawei.com> 抄送: Jens Axboe <axboe@kernel.dk>; linux-block@vger.kernel.org; linux-kernel@vger.kernel.org 主题: Re: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request > Il giorno 16 dic 2021, alle ore 13:21, Zhang Wensheng <zhangwensheng5@huawei.com> ha scritto: > > KASAN reports a use-after-free report when doing normal scsi-mq test > > [69832.239032] > ================================================================== > [69832.241810] BUG: KASAN: use-after-free in > bfq_dispatch_request+0x1045/0x44b0 > [69832.243267] Read of size 8 at addr ffff88802622ba88 by task > kworker/3:1H/155 [69832.244656] [69832.245007] CPU: 3 PID: 155 Comm: > kworker/3:1H Not tainted 5.10.0-10295-g576c6382529e #8 [69832.246626] > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014 [69832.249069] > Workqueue: kblockd blk_mq_run_work_fn [69832.250022] Call Trace: > [69832.250541] dump_stack+0x9b/0xce > [69832.251232] ? bfq_dispatch_request+0x1045/0x44b0 > [69832.252243] print_address_description.constprop.6+0x3e/0x60 > [69832.253381] ? __cpuidle_text_end+0x5/0x5 [69832.254211] ? > vprintk_func+0x6b/0x120 [69832.254994] ? > bfq_dispatch_request+0x1045/0x44b0 > [69832.255952] ? bfq_dispatch_request+0x1045/0x44b0 > [69832.256914] kasan_report.cold.9+0x22/0x3a [69832.257753] ? > bfq_dispatch_request+0x1045/0x44b0 > [69832.258755] check_memory_region+0x1c1/0x1e0 [69832.260248] > bfq_dispatch_request+0x1045/0x44b0 > [69832.261181] ? bfq_bfqq_expire+0x2440/0x2440 [69832.262032] ? > blk_mq_delay_run_hw_queues+0xf9/0x170 > [69832.263022] __blk_mq_do_dispatch_sched+0x52f/0x830 > [69832.264011] ? blk_mq_sched_request_inserted+0x100/0x100 > [69832.265101] __blk_mq_sched_dispatch_requests+0x398/0x4f0 > [69832.266206] ? blk_mq_do_dispatch_ctx+0x570/0x570 > [69832.267147] ? __switch_to+0x5f4/0xee0 [69832.267898] > blk_mq_sched_dispatch_requests+0xdf/0x140 > [69832.268946] __blk_mq_run_hw_queue+0xc0/0x270 [69832.269840] > blk_mq_run_work_fn+0x51/0x60 [69832.278170] > process_one_work+0x6d4/0xfe0 [69832.278984] worker_thread+0x91/0xc80 > [69832.279726] ? __kthread_parkme+0xb0/0x110 [69832.280554] ? > process_one_work+0xfe0/0xfe0 [69832.281414] kthread+0x32d/0x3f0 > [69832.282082] ? kthread_park+0x170/0x170 [69832.282849] > ret_from_fork+0x1f/0x30 [69832.283573] [69832.283886] Allocated by > task 7725: > [69832.284599] kasan_save_stack+0x19/0x40 [69832.285385] > __kasan_kmalloc.constprop.2+0xc1/0xd0 > [69832.286350] kmem_cache_alloc_node+0x13f/0x460 [69832.287237] > bfq_get_queue+0x3d4/0x1140 [69832.287993] > bfq_get_bfqq_handle_split+0x103/0x510 > [69832.289015] bfq_init_rq+0x337/0x2d50 [69832.289749] > bfq_insert_requests+0x304/0x4e10 [69832.290634] > blk_mq_sched_insert_requests+0x13e/0x390 > [69832.291629] blk_mq_flush_plug_list+0x4b4/0x760 > [69832.292538] blk_flush_plug_list+0x2c5/0x480 [69832.293392] > io_schedule_prepare+0xb2/0xd0 [69832.294209] > io_schedule_timeout+0x13/0x80 [69832.295014] > wait_for_common_io.constprop.1+0x13c/0x270 > [69832.296137] submit_bio_wait+0x103/0x1a0 [69832.296932] > blkdev_issue_discard+0xe6/0x160 [69832.297794] > blk_ioctl_discard+0x219/0x290 [69832.298614] > blkdev_common_ioctl+0x50a/0x1750 [69832.304715] > blkdev_ioctl+0x470/0x600 [69832.305474] block_ioctl+0xde/0x120 > [69832.306232] vfs_ioctl+0x6c/0xc0 [69832.306877] > __se_sys_ioctl+0x90/0xa0 [69832.307629] do_syscall_64+0x2d/0x40 > [69832.308362] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [69832.309382] > [69832.309701] Freed by task 155: > [69832.310328] kasan_save_stack+0x19/0x40 [69832.311121] > kasan_set_track+0x1c/0x30 [69832.311868] > kasan_set_free_info+0x1b/0x30 [69832.312699] > __kasan_slab_free+0x111/0x160 [69832.313524] > kmem_cache_free+0x94/0x460 [69832.314367] bfq_put_queue+0x582/0x940 > [69832.315112] __bfq_bfqd_reset_in_service+0x166/0x1d0 > [69832.317275] bfq_bfqq_expire+0xb27/0x2440 [69832.318084] > bfq_dispatch_request+0x697/0x44b0 [69832.318991] > __blk_mq_do_dispatch_sched+0x52f/0x830 > [69832.319984] __blk_mq_sched_dispatch_requests+0x398/0x4f0 > [69832.321087] blk_mq_sched_dispatch_requests+0xdf/0x140 > [69832.322225] __blk_mq_run_hw_queue+0xc0/0x270 [69832.323114] > blk_mq_run_work_fn+0x51/0x60 [69832.323942] > process_one_work+0x6d4/0xfe0 [69832.324772] worker_thread+0x91/0xc80 > [69832.325518] kthread+0x32d/0x3f0 [69832.326205] > ret_from_fork+0x1f/0x30 [69832.326932] [69832.338297] The buggy > address belongs to the object at ffff88802622b968 [69832.338297] > which belongs to the cache bfq_queue of size 512 [69832.340766] The > buggy address is located 288 bytes inside of [69832.340766] 512-byte > region [ffff88802622b968, ffff88802622bb68) [69832.343091] The buggy > address belongs to the page: > [69832.344097] page:ffffea0000988a00 refcount:1 mapcount:0 > mapping:0000000000000000 index:0xffff88802622a528 pfn:0x26228 > [69832.346214] head:ffffea0000988a00 order:2 compound_mapcount:0 > compound_pincount:0 [69832.347719] flags: 0x1fffff80010200(slab|head) > [69832.348625] raw: 001fffff80010200 ffffea0000dbac08 ffff888017a57650 > ffff8880179fe840 [69832.354972] raw: ffff88802622a528 0000000000120008 > 00000001ffffffff 0000000000000000 [69832.356547] page dumped because: > kasan: bad access detected [69832.357652] [69832.357970] Memory state around the buggy address: > [69832.358926] ffff88802622b980: fb fb fb fb fb fb fb fb fb fb fb fb > fb fb fb fb [69832.360358] ffff88802622ba00: fb fb fb fb fb fb fb fb > fb fb fb fb fb fb fb fb [69832.361810] >ffff88802622ba80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > [69832.363273] ^ > [69832.363975] ffff88802622bb00: fb fb fb fb fb fb fb fb fb fb fb fb > fb fc fc fc [69832.375960] ffff88802622bb80: fc fc fc fc fc fc fc fc > fc fc fc fc fc fc fc fc [69832.377405] > ================================================================== > > In bfq_dispatch_requestfunction, it may have function call: > > bfq_dispatch_request > __bfq_dispatch_request > bfq_select_queue > bfq_bfqq_expire > __bfq_bfqd_reset_in_service > bfq_put_queue > kmem_cache_free > In this function call, in_serv_queue has beed expired and meet the > conditions to free. In the function bfq_dispatch_request, the address > of in_serv_queue pointing to has been released. For getting the value > of idle_timer_disabled, it will get flags value from the address which > in_serv_queue pointing to, then the problem of use-after-free happens; > > Fix the problem by check in_serv_queue == bfqd->in_service_queue, to > get the value of idle_timer_disabled if in_serve_queue is equel to > bfqd->in_service_queue. If the space of in_serv_queue pointing has > been released, this judge will aviod use-after-free problem. > And if in_serv_queue may be expired but it still exists, this judge > may have little effects on the function bfqg_stats_update_idle_time in > bfq_update_dispatch_stats. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Wensheng <zhangwensheng5@huawei.com> > --- > block/bfq-iosched.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index > fec18118dc30..97533634b99e 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -5061,7 +5061,7 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) > struct bfq_data *bfqd = hctx->queue->elevator->elevator_data; > struct request *rq; > struct bfq_queue *in_serv_queue; > - bool waiting_rq, idle_timer_disabled; > + bool waiting_rq, idle_timer_disabled = false; > > spin_lock_irq(&bfqd->lock); > > @@ -5070,9 +5070,10 @@ static struct request > *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) > > rq = __bfq_dispatch_request(hctx); > > - idle_timer_disabled = > - waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); > - > + if (in_serv_queue == bfqd->in_service_queue) { > + idle_timer_disabled = > + waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); > + } Good catch! > spin_unlock_irq(&bfqd->lock); > > bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue, Yet, what about the above use of in_serv_queue then? Thanks, Paolo > -- > 2.31.1 >