From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Bart Van Assche , Christoph Hellwig , Robert Elliott , Ming Lei , Alexander Gordeev , Jens Axboe Subject: [PATCH 3.18 047/150] blk-mq: Fix a use-after-free Date: Tue, 13 Jan 2015 23:21:58 -0800 Message-Id: <20150114072058.010672123@linuxfoundation.org> In-Reply-To: <20150114072055.842408181@linuxfoundation.org> References: <20150114072055.842408181@linuxfoundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Sender: linux-kernel-owner@vger.kernel.org List-ID: 3.18-stable review patch. If anyone has any objections, please let me know. ------------------ From: Bart Van Assche commit 45a9c9d909b24c6ad0e28a7946e7486e73010319 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: [] lock_acquire+0xc4/0x270 [] mutex_lock_nested+0x61/0x380 [] blk_mq_free_queue+0x30/0x180 [] blk_release_queue+0x84/0xd0 [] kobject_cleanup+0x7b/0x1a0 [] kobject_put+0x30/0x70 [] blk_put_queue+0x15/0x20 [] disk_release+0x99/0xd0 [] device_release+0x36/0xb0 [] kobject_cleanup+0x7b/0x1a0 [] kobject_put+0x30/0x70 [] put_disk+0x1a/0x20 [] __blkdev_put+0x135/0x1b0 [] blkdev_put+0x50/0x160 [] kill_block_super+0x44/0x70 [] deactivate_locked_super+0x44/0x60 [] deactivate_super+0x4e/0x70 [] cleanup_mnt+0x43/0x90 [] __cleanup_mnt+0x12/0x20 [] task_work_run+0xac/0xe0 [] do_notify_resume+0x61/0xa0 [] int_signal+0x12/0x17 Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Robert Elliott Cc: Ming Lei Cc: Alexander Gordeev Signed-off-by: Jens Axboe Signed-off-by: Greg Kroah-Hartman --- block/blk-core.c | 3 +++ block/blk-sysfs.c | 12 ++++-------- 2 files changed, 7 insertions(+), 8 deletions(-) --- a/block/blk-core.c +++ b/block/blk-core.c @@ -525,6 +525,9 @@ void blk_cleanup_queue(struct request_qu del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer); blk_sync_queue(q); + if (q->mq_ops) + blk_mq_free_queue(q); + spin_lock_irq(lock); if (q->queue_lock != &q->__queue_lock) q->queue_lock = &q->__queue_lock; --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -492,17 +492,15 @@ static void blk_free_queue_rcu(struct rc * Currently, its primary task it to free all the &struct request * structures that were allocated to the queue and the queue itself. * - * Caveat: - * Hopefully the low level driver will have finished any - * outstanding requests first... + * Note: + * The low level driver must have finished any outstanding requests first + * via blk_cleanup_queue(). **/ static void blk_release_queue(struct kobject *kobj) { struct request_queue *q = container_of(kobj, struct request_queue, kobj); - blk_sync_queue(q); - blkcg_exit_queue(q); if (q->elevator) { @@ -517,9 +515,7 @@ static void blk_release_queue(struct kob if (q->queue_tags) __blk_queue_free_tags(q); - if (q->mq_ops) - blk_mq_free_queue(q); - else + if (!q->mq_ops) blk_free_flush_queue(q->fq); blk_trace_shutdown(q);