* [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
@ 2021-12-16 12:21 Zhang Wensheng
2021-12-16 16:27 ` Paolo Valente
0 siblings, 1 reply; 7+ messages in thread
From: Zhang Wensheng @ 2021-12-16 12:21 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel
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);
+ }
spin_unlock_irq(&bfqd->lock);
bfq_update_dispatch_stats(hctx->queue, rq, in_serv_queue,
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-16 12:21 [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request Zhang Wensheng
@ 2021-12-16 16:27 ` Paolo Valente
2021-12-20 2:35 ` 答复: " zhangwensheng (E)
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Paolo Valente @ 2021-12-16 16:27 UTC (permalink / raw)
To: Zhang Wensheng; +Cc: Jens Axboe, linux-block, linux-kernel
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-16 16:27 ` Paolo Valente
@ 2021-12-20 2:35 ` zhangwensheng (E)
2021-12-23 2:06 ` zhangwensheng (E)
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: zhangwensheng (E) @ 2021-12-20 2:35 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, linux-kernel
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(+)
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
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-16 16:27 ` Paolo Valente
2021-12-20 2:35 ` 答复: " zhangwensheng (E)
@ 2021-12-23 2:06 ` zhangwensheng (E)
2022-01-19 13:49 ` Paolo Valente
2022-01-04 8:58 ` 答复: " zhangwensheng (E)
2022-01-19 2:39 ` zhangwensheng (E)
3 siblings, 1 reply; 7+ messages in thread
From: zhangwensheng (E) @ 2021-12-23 2:06 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, linux-kernel
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(+)
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-16 16:27 ` Paolo Valente
2021-12-20 2:35 ` 答复: " zhangwensheng (E)
2021-12-23 2:06 ` zhangwensheng (E)
@ 2022-01-04 8:58 ` zhangwensheng (E)
2022-01-19 2:39 ` zhangwensheng (E)
3 siblings, 0 replies; 7+ messages in thread
From: zhangwensheng (E) @ 2022-01-04 8:58 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, linux-kernel
Ping...
-----邮件原件-----
发件人: zhangwensheng (E)
发送时间: 2021年12月23日 10:06
收件人: 'Paolo Valente' <paolo.valente@linaro.org>
抄送: 'Jens Axboe' <axboe@kernel.dk>; 'linux-block@vger.kernel.org' <linux-block@vger.kernel.org>; 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>
主题: 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
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(+)
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-16 16:27 ` Paolo Valente
` (2 preceding siblings ...)
2022-01-04 8:58 ` 答复: " zhangwensheng (E)
@ 2022-01-19 2:39 ` zhangwensheng (E)
3 siblings, 0 replies; 7+ messages in thread
From: zhangwensheng (E) @ 2022-01-19 2:39 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, linux-block, linux-kernel
Ping...
-----邮件原件-----
发件人: zhangwensheng (E)
发送时间: 2022年1月4日 16:58
收件人: 'Paolo Valente' <paolo.valente@linaro.org>
抄送: 'Jens Axboe' <axboe@kernel.dk>; 'linux-block@vger.kernel.org' <linux-block@vger.kernel.org>; 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>
主题: 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
Ping...
-----邮件原件-----
发件人: zhangwensheng (E)
发送时间: 2021年12月23日 10:06
收件人: 'Paolo Valente' <paolo.valente@linaro.org>
抄送: 'Jens Axboe' <axboe@kernel.dk>; 'linux-block@vger.kernel.org' <linux-block@vger.kernel.org>; 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>
主题: 答复: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
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(+)
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request
2021-12-23 2:06 ` zhangwensheng (E)
@ 2022-01-19 13:49 ` Paolo Valente
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Valente @ 2022-01-19 13:49 UTC (permalink / raw)
To: zhangwensheng (E); +Cc: Jens Axboe, linux-block, linux-kernel
> 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
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-19 13:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 12:21 [PATCH -next v3] bfq: fix use-after-free in bfq_dispatch_request Zhang Wensheng
2021-12-16 16:27 ` Paolo Valente
2021-12-20 2:35 ` 答复: " zhangwensheng (E)
2021-12-23 2:06 ` zhangwensheng (E)
2022-01-19 13:49 ` Paolo Valente
2022-01-04 8:58 ` 答复: " zhangwensheng (E)
2022-01-19 2:39 ` zhangwensheng (E)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.