Commit 8fe4ce58 authored by Bart Van Assche's avatar Bart Van Assche Committed by Martin K. Petersen

scsi: core: Fix a use-after-free

There are two .exit_cmd_priv implementations. Both implementations use
resources associated with the SCSI host. Make sure that these resources are
still available when .exit_cmd_priv is called by waiting inside
scsi_remove_host() until the tag set has been freed.

This commit fixes the following use-after-free:

==================================================================
BUG: KASAN: use-after-free in srp_exit_cmd_priv+0x27/0xd0 [ib_srp]
Read of size 8 at addr ffff888100337000 by task multipathd/16727
Call Trace:
 <TASK>
 dump_stack_lvl+0x34/0x44
 print_report.cold+0x5e/0x5db
 kasan_report+0xab/0x120
 srp_exit_cmd_priv+0x27/0xd0 [ib_srp]
 scsi_mq_exit_request+0x4d/0x70
 blk_mq_free_rqs+0x143/0x410
 __blk_mq_free_map_and_rqs+0x6e/0x100
 blk_mq_free_tag_set+0x2b/0x160
 scsi_host_dev_release+0xf3/0x1a0
 device_release+0x54/0xe0
 kobject_put+0xa5/0x120
 device_release+0x54/0xe0
 kobject_put+0xa5/0x120
 scsi_device_dev_release_usercontext+0x4c1/0x4e0
 execute_in_process_context+0x23/0x90
 device_release+0x54/0xe0
 kobject_put+0xa5/0x120
 scsi_disk_release+0x3f/0x50
 device_release+0x54/0xe0
 kobject_put+0xa5/0x120
 disk_release+0x17f/0x1b0
 device_release+0x54/0xe0
 kobject_put+0xa5/0x120
 dm_put_table_device+0xa3/0x160 [dm_mod]
 dm_put_device+0xd0/0x140 [dm_mod]
 free_priority_group+0xd8/0x110 [dm_multipath]
 free_multipath+0x94/0xe0 [dm_multipath]
 dm_table_destroy+0xa2/0x1e0 [dm_mod]
 __dm_destroy+0x196/0x350 [dm_mod]
 dev_remove+0x10c/0x160 [dm_mod]
 ctl_ioctl+0x2c2/0x590 [dm_mod]
 dm_ctl_ioctl+0x5/0x10 [dm_mod]
 __x64_sys_ioctl+0xb4/0xf0
 dm_ctl_ioctl+0x5/0x10 [dm_mod]
 __x64_sys_ioctl+0xb4/0xf0
 do_syscall_64+0x3b/0x90
 entry_SYSCALL_64_after_hwframe+0x46/0xb0

Link: https://lore.kernel.org/r/20220826002635.919423-1-bvanassche@acm.org
Fixes: 65ca846a ("scsi: core: Introduce {init,exit}_cmd_priv()")
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.garry@huawei.com>
Cc: Li Zhijian <lizhijian@fujitsu.com>
Reported-by: default avatarLi Zhijian <lizhijian@fujitsu.com>
Tested-by: default avatarLi Zhijian <lizhijian@fujitsu.com>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent f782201e
...@@ -182,6 +182,15 @@ void scsi_remove_host(struct Scsi_Host *shost) ...@@ -182,6 +182,15 @@ void scsi_remove_host(struct Scsi_Host *shost)
mutex_unlock(&shost->scan_mutex); mutex_unlock(&shost->scan_mutex);
scsi_proc_host_rm(shost); scsi_proc_host_rm(shost);
/*
* New SCSI devices cannot be attached anymore because of the SCSI host
* state so drop the tag set refcnt. Wait until the tag set refcnt drops
* to zero because .exit_cmd_priv implementations may need the host
* pointer.
*/
kref_put(&shost->tagset_refcnt, scsi_mq_free_tags);
wait_for_completion(&shost->tagset_freed);
spin_lock_irqsave(shost->host_lock, flags); spin_lock_irqsave(shost->host_lock, flags);
if (scsi_host_set_state(shost, SHOST_DEL)) if (scsi_host_set_state(shost, SHOST_DEL))
BUG_ON(scsi_host_set_state(shost, SHOST_DEL_RECOVERY)); BUG_ON(scsi_host_set_state(shost, SHOST_DEL_RECOVERY));
...@@ -245,6 +254,9 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, ...@@ -245,6 +254,9 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
if (error) if (error)
goto fail; goto fail;
kref_init(&shost->tagset_refcnt);
init_completion(&shost->tagset_freed);
/* /*
* Increase usage count temporarily here so that calling * Increase usage count temporarily here so that calling
* scsi_autopm_put_host() will trigger runtime idle if there is * scsi_autopm_put_host() will trigger runtime idle if there is
...@@ -317,6 +329,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, ...@@ -317,6 +329,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
pm_runtime_disable(&shost->shost_gendev); pm_runtime_disable(&shost->shost_gendev);
pm_runtime_set_suspended(&shost->shost_gendev); pm_runtime_set_suspended(&shost->shost_gendev);
pm_runtime_put_noidle(&shost->shost_gendev); pm_runtime_put_noidle(&shost->shost_gendev);
kref_put(&shost->tagset_refcnt, scsi_mq_free_tags);
fail: fail:
return error; return error;
} }
...@@ -350,9 +363,6 @@ static void scsi_host_dev_release(struct device *dev) ...@@ -350,9 +363,6 @@ static void scsi_host_dev_release(struct device *dev)
kfree(dev_name(&shost->shost_dev)); kfree(dev_name(&shost->shost_dev));
} }
if (shost->tag_set.tags)
scsi_mq_destroy_tags(shost);
kfree(shost->shost_data); kfree(shost->shost_data);
ida_free(&host_index_ida, shost->host_no); ida_free(&host_index_ida, shost->host_no);
......
...@@ -1983,9 +1983,13 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) ...@@ -1983,9 +1983,13 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
return blk_mq_alloc_tag_set(tag_set); return blk_mq_alloc_tag_set(tag_set);
} }
void scsi_mq_destroy_tags(struct Scsi_Host *shost) void scsi_mq_free_tags(struct kref *kref)
{ {
struct Scsi_Host *shost = container_of(kref, typeof(*shost),
tagset_refcnt);
blk_mq_free_tag_set(&shost->tag_set); blk_mq_free_tag_set(&shost->tag_set);
complete(&shost->tagset_freed);
} }
/** /**
......
...@@ -94,7 +94,7 @@ extern void scsi_run_host_queues(struct Scsi_Host *shost); ...@@ -94,7 +94,7 @@ extern void scsi_run_host_queues(struct Scsi_Host *shost);
extern void scsi_requeue_run_queue(struct work_struct *work); extern void scsi_requeue_run_queue(struct work_struct *work);
extern void scsi_start_queue(struct scsi_device *sdev); extern void scsi_start_queue(struct scsi_device *sdev);
extern int scsi_mq_setup_tags(struct Scsi_Host *shost); extern int scsi_mq_setup_tags(struct Scsi_Host *shost);
extern void scsi_mq_destroy_tags(struct Scsi_Host *shost); extern void scsi_mq_free_tags(struct kref *kref);
extern void scsi_exit_queue(void); extern void scsi_exit_queue(void);
extern void scsi_evt_thread(struct work_struct *work); extern void scsi_evt_thread(struct work_struct *work);
......
...@@ -340,6 +340,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, ...@@ -340,6 +340,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
kfree(sdev); kfree(sdev);
goto out; goto out;
} }
kref_get(&sdev->host->tagset_refcnt);
sdev->request_queue = q; sdev->request_queue = q;
q->queuedata = sdev; q->queuedata = sdev;
__scsi_init_queue(sdev->host, q); __scsi_init_queue(sdev->host, q);
......
...@@ -1476,6 +1476,7 @@ void __scsi_remove_device(struct scsi_device *sdev) ...@@ -1476,6 +1476,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
mutex_unlock(&sdev->state_mutex); mutex_unlock(&sdev->state_mutex);
blk_mq_destroy_queue(sdev->request_queue); blk_mq_destroy_queue(sdev->request_queue);
kref_put(&sdev->host->tagset_refcnt, scsi_mq_free_tags);
cancel_work_sync(&sdev->requeue_work); cancel_work_sync(&sdev->requeue_work);
if (sdev->host->hostt->slave_destroy) if (sdev->host->hostt->slave_destroy)
......
...@@ -557,6 +557,8 @@ struct Scsi_Host { ...@@ -557,6 +557,8 @@ struct Scsi_Host {
struct scsi_host_template *hostt; struct scsi_host_template *hostt;
struct scsi_transport_template *transportt; struct scsi_transport_template *transportt;
struct kref tagset_refcnt;
struct completion tagset_freed;
/* Area to keep a shared tag map */ /* Area to keep a shared tag map */
struct blk_mq_tag_set tag_set; struct blk_mq_tag_set tag_set;
......
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