Re: [PATCH -next] blk-throttle: Set BIO_THROTTLED when bio has been throttled
From: QiuLaibin
Date: Mon Dec 13 2021 - 04:24:08 EST
Hi Tejun:
Thanks for your reply.
And the following is the stack of this problem:
[112105.756554] BUG: KASAN: use-after-free in blk_throtl_bio+0x12f0/0x2c70
[112105.763367] Call Trace:
[112105.763889] dump_stack+0x9b/0xce
[112105.766343] print_address_description.constprop.6+0x3e/0x60
[112105.778343] kasan_report.cold.9+0x22/0x3a
[112105.780984] blk_throtl_bio+0x12f0/0x2c70
[112105.782759] submit_bio_checks+0x701/0x1550
[112105.803063] submit_bio_noacct+0x83/0xc80
[112105.807930] submit_bio+0xa7/0x330
[112105.808649] mpage_readahead+0x380/0x500
[112105.812982] read_pages+0x1c1/0xbf0
[112105.817575] page_cache_ra_unbounded+0x471/0x6f0
[112105.820014] do_page_cache_ra+0xda/0x110
[112105.824801] ondemand_readahead+0x442/0xae0
[112105.825631] page_cache_async_ra+0x210/0x300
[112105.826487] generic_file_buffered_read+0x4d9/0x2130
[112105.829308] generic_file_read_iter+0x315/0x490
[112105.831073] blkdev_read_iter+0x113/0x1b0
[112105.831885] aio_read+0x2ad/0x450
[112105.867371] io_submit_one+0xc8e/0x1d60
[112105.873391] __se_sys_io_submit+0x125/0x350
[112105.893371] do_syscall_64+0x2d/0x40
[112105.894072] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[112105.925135] Allocated by task 26380:
[112105.925866] kasan_save_stack+0x19/0x40
[112105.926666] __kasan_kmalloc.constprop.2+0xc1/0xd0
[112105.927644] kmem_cache_alloc+0x146/0x440
[112105.928704] mempool_alloc+0x125/0x2f0
[112105.929464] bio_alloc_bioset+0x353/0x590
[112105.930227] mpage_alloc+0x3b/0x240
[112105.932013] do_mpage_readpage+0xddf/0x1ef0
[112105.933302] mpage_readahead+0x264/0x500
[112105.934092] read_pages+0x1c1/0xbf0
[112105.934786] page_cache_ra_unbounded+0x471/0x6f0
[112105.935731] do_page_cache_ra+0xda/0x110
[112105.936542] ondemand_readahead+0x442/0xae0
[112105.937391] page_cache_async_ra+0x210/0x300
[112105.938269] generic_file_buffered_read+0x4d9/0x2130
[112105.939542] generic_file_read_iter+0x315/0x490
[112105.940539] blkdev_read_iter+0x113/0x1b0
[112105.941368] aio_read+0x2ad/0x450
[112105.942129] io_submit_one+0xc8e/0x1d60
[112105.942895] __se_sys_io_submit+0x125/0x350
[112105.943713] do_syscall_64+0x2d/0x40
[112105.948600] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[112105.951025] Freed by task 0:
[112105.951575] kasan_save_stack+0x19/0x40
[112105.952293] kasan_set_track+0x1c/0x30
[112105.952998] kasan_set_free_info+0x1b/0x30
[112105.953792] __kasan_slab_free+0x111/0x160
[112105.955805] kmem_cache_free+0x94/0x460
[112105.956724] mempool_free+0xd6/0x320
[112105.957667] bio_free+0xe0/0x130
[112105.958299] bio_put+0xab/0xe0
[112105.958901] bio_endio+0x3a6/0x5d0
[112105.959577] blk_update_request+0x590/0x1370
[112105.964202] scsi_end_request+0x7d/0x400
[112105.967260] scsi_io_completion+0x1aa/0xe50
[112105.968064] scsi_softirq_done+0x11b/0x240
[112105.968835] blk_mq_complete_request+0xd4/0x120
[112105.969734] scsi_mq_done+0xf0/0x200
[112105.974126] virtscsi_vq_done+0xbc/0x150
[112105.974904] vring_interrupt+0x179/0x390
[112105.975718] __handle_irq_event_percpu+0xf7/0x490
[112105.976630] handle_irq_event_percpu+0x7b/0x160
[112105.990310] handle_irq_event+0xcc/0x170
[112105.993845] handle_edge_irq+0x215/0xb20
[112105.994610] common_interrupt+0x60/0x120
[112106.006560] asm_common_interrupt+0x1e/0x40
On 2021/11/30 2:16, Tejun Heo wrote:
Hello,
On Thu, Nov 18, 2021 at 09:15:51PM +0800, Laibin Qiu wrote:
1.In current process, all bio will set the BIO_THROTTLED flag
after __blk_throtl_bio().
2.If bio needs to be throttled, it will start the timer and
stop submit bio directly. Bio will submit in blk_throtl_dispatch_work_fn()
when the timer expires. But in the current process, if bio is throttled.
The BIO_THROTTLED will be set to bio after timer start. If the bio
has been completed, it may cause use-after-free.
Fix this by move BIO_THROTTLED set before timer set.
Have you tried reproducing and confirming the above in any way?
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 39bb6e68a9a2..ddfbff4465d5 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2149,6 +2149,7 @@ bool __blk_throtl_bio(struct bio *bio)
td->nr_queued[rw]++;
throtl_add_bio_tg(bio, qn, tg);
throttled = true;
+ bio_set_flag(bio, BIO_THROTTLED);
/*
* Update @tg's dispatch time and force schedule dispatch if @tg
@@ -2163,7 +2164,6 @@ bool __blk_throtl_bio(struct bio *bio)
out_unlock:
spin_unlock_irq(&q->queue_lock);
- bio_set_flag(bio, BIO_THROTTLED);
Because it seems wrong in two ways:
* This function is called synchronously on the issue path. The bio isn't
seen by the queue and device driver yet and nothing can race to issue it
before this function returns.
The bio is under throttle here, this submit_bio return directly. And
current process will queue a dispatch work by
throtl_schedule_pending_timer() to submit this bio before BIO_THROTTLED
flag set. If the bio is completed quickly after the dispatch work is
queued, UAF of bio will happen.
* Now we're not setting BIO_THROTTLED when we're taking a different return
path through the out_unlock label and risks calling back into blk_throtl
again on the same bio.
In my opinion, This flag can prevent the request from being throttled
multiple times. If the request itself does not need to be throttled, the
result of repeated entry will be the same.
If necessary, I think we can use other methods to achieve this effect
for request does not need to be throttled.
In general, if you think you spotted an issue, please try to trigger it in
however way possible to confirm that the issue is real.
Thanks.
Best regards