Commit 294d7496 authored by Bart Van Assche's avatar Bart Van Assche Committed by Luis Henriques

blk-mq: Fix a use-after-free

commit 45a9c9d9 upstream.

blk-mq users are allowed to free the memory request_queue.tag_set
points at after blk_cleanup_queue() has finished but before
blk_release_queue() has started. This can happen e.g. in the SCSI
core. The SCSI core namely embeds the tag_set structure in a SCSI
host structure. The SCSI host structure is freed by
scsi_host_dev_release(). This function is called after
blk_cleanup_queue() finished but can be called before
blk_release_queue().

This means that it is not safe to access request_queue.tag_set from
inside blk_release_queue(). Hence remove the blk_sync_queue() call
from blk_release_queue(). This call is not necessary - outstanding
requests must have finished before blk_release_queue() is
called. Additionally, move the blk_mq_free_queue() call from
blk_release_queue() to blk_cleanup_queue() to avoid that struct
request_queue.tag_set gets accessed after it has been freed.

This patch avoids that the following kernel oops can be triggered
when deleting a SCSI host for which scsi-mq was enabled:

Call Trace:
 [<ffffffff8109a7c4>] lock_acquire+0xc4/0x270
 [<ffffffff814ce111>] mutex_lock_nested+0x61/0x380
 [<ffffffff812575f0>] blk_mq_free_queue+0x30/0x180
 [<ffffffff8124d654>] blk_release_queue+0x84/0xd0
 [<ffffffff8126c29b>] kobject_cleanup+0x7b/0x1a0
 [<ffffffff8126c140>] kobject_put+0x30/0x70
 [<ffffffff81245895>] blk_put_queue+0x15/0x20
 [<ffffffff8125c409>] disk_release+0x99/0xd0
 [<ffffffff8133d056>] device_release+0x36/0xb0
 [<ffffffff8126c29b>] kobject_cleanup+0x7b/0x1a0
 [<ffffffff8126c140>] kobject_put+0x30/0x70
 [<ffffffff8125a78a>] put_disk+0x1a/0x20
 [<ffffffff811d4cb5>] __blkdev_put+0x135/0x1b0
 [<ffffffff811d56a0>] blkdev_put+0x50/0x160
 [<ffffffff81199eb4>] kill_block_super+0x44/0x70
 [<ffffffff8119a2a4>] deactivate_locked_super+0x44/0x60
 [<ffffffff8119a87e>] deactivate_super+0x4e/0x70
 [<ffffffff811b9833>] cleanup_mnt+0x43/0x90
 [<ffffffff811b98d2>] __cleanup_mnt+0x12/0x20
 [<ffffffff8107252c>] task_work_run+0xac/0xe0
 [<ffffffff81002c01>] do_notify_resume+0x61/0xa0
 [<ffffffff814d2c58>] int_signal+0x12/0x17
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Robert Elliott <elliott@hp.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Alexander Gordeev <agordeev@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@fb.com>
[ luis: backported to 3.16: adjusted context ]
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 59f4945d
...@@ -524,6 +524,9 @@ void blk_cleanup_queue(struct request_queue *q) ...@@ -524,6 +524,9 @@ void blk_cleanup_queue(struct request_queue *q)
del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer); del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
blk_sync_queue(q); blk_sync_queue(q);
if (q->mq_ops)
blk_mq_free_queue(q);
spin_lock_irq(lock); spin_lock_irq(lock);
if (q->queue_lock != &q->__queue_lock) if (q->queue_lock != &q->__queue_lock)
q->queue_lock = &q->__queue_lock; q->queue_lock = &q->__queue_lock;
......
...@@ -492,17 +492,15 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head) ...@@ -492,17 +492,15 @@ static void blk_free_queue_rcu(struct rcu_head *rcu_head)
* Currently, its primary task it to free all the &struct request * Currently, its primary task it to free all the &struct request
* structures that were allocated to the queue and the queue itself. * structures that were allocated to the queue and the queue itself.
* *
* Caveat: * Note:
* Hopefully the low level driver will have finished any * The low level driver must have finished any outstanding requests first
* outstanding requests first... * via blk_cleanup_queue().
**/ **/
static void blk_release_queue(struct kobject *kobj) static void blk_release_queue(struct kobject *kobj)
{ {
struct request_queue *q = struct request_queue *q =
container_of(kobj, struct request_queue, kobj); container_of(kobj, struct request_queue, kobj);
blk_sync_queue(q);
blkcg_exit_queue(q); blkcg_exit_queue(q);
if (q->elevator) { if (q->elevator) {
...@@ -517,9 +515,6 @@ static void blk_release_queue(struct kobject *kobj) ...@@ -517,9 +515,6 @@ static void blk_release_queue(struct kobject *kobj)
if (q->queue_tags) if (q->queue_tags)
__blk_queue_free_tags(q); __blk_queue_free_tags(q);
if (q->mq_ops)
blk_mq_free_queue(q);
kfree(q->flush_rq); kfree(q->flush_rq);
blk_trace_shutdown(q); blk_trace_shutdown(q);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment