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

scsi: core: Make sure that targets outlive devices

This commit prevents that the following sequence triggers a kernel crash:

 - Deletion of a SCSI device is requested via sysfs. Device removal takes
   some time because blk_cleanup_queue() is waiting for the SCSI error
   handler.

 - The SCSI target associated with that SCSI device is removed.

 - scsi_remove_target() returns and its caller frees the resources
   associated with the SCSI target.

 - The error handler makes progress and invokes an LLD callback that
   dereferences the SCSI target pointer.

Link: https://lore.kernel.org/r/20220728221851.1822295-2-bvanassche@acm.org
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 avatarMike Christie <michael.christie@oracle.com>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Reviewed-by: default avatarMike Christie <michael.christie@oracle.com>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent a1906678
...@@ -521,6 +521,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, ...@@ -521,6 +521,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
starget->state = STARGET_CREATED; starget->state = STARGET_CREATED;
starget->scsi_level = SCSI_2; starget->scsi_level = SCSI_2;
starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED; starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
init_waitqueue_head(&starget->sdev_wq);
retry: retry:
spin_lock_irqsave(shost->host_lock, flags); spin_lock_irqsave(shost->host_lock, flags);
......
...@@ -443,7 +443,9 @@ static void scsi_device_cls_release(struct device *class_dev) ...@@ -443,7 +443,9 @@ static void scsi_device_cls_release(struct device *class_dev)
static void scsi_device_dev_release_usercontext(struct work_struct *work) static void scsi_device_dev_release_usercontext(struct work_struct *work)
{ {
struct scsi_device *sdev; struct scsi_device *sdev = container_of(work, struct scsi_device,
ew.work);
struct scsi_target *starget = sdev->sdev_target;
struct device *parent; struct device *parent;
struct list_head *this, *tmp; struct list_head *this, *tmp;
struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL; struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL;
...@@ -452,8 +454,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) ...@@ -452,8 +454,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
unsigned long flags; unsigned long flags;
struct module *mod; struct module *mod;
sdev = container_of(work, struct scsi_device, ew.work);
mod = sdev->host->hostt->module; mod = sdev->host->hostt->module;
scsi_dh_release_device(sdev); scsi_dh_release_device(sdev);
...@@ -516,6 +516,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) ...@@ -516,6 +516,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
kfree(sdev->inquiry); kfree(sdev->inquiry);
kfree(sdev); kfree(sdev);
if (starget && atomic_dec_return(&starget->sdev_count) == 0)
wake_up(&starget->sdev_wq);
if (parent) if (parent)
put_device(parent); put_device(parent);
module_put(mod); module_put(mod);
...@@ -1535,6 +1538,14 @@ static void __scsi_remove_target(struct scsi_target *starget) ...@@ -1535,6 +1538,14 @@ static void __scsi_remove_target(struct scsi_target *starget)
goto restart; goto restart;
} }
spin_unlock_irqrestore(shost->host_lock, flags); spin_unlock_irqrestore(shost->host_lock, flags);
/*
* After scsi_remove_target() returns its caller can remove resources
* associated with @starget, e.g. an rport or session. Wait until all
* devices associated with @starget have been removed to prevent that
* a SCSI error handling callback function triggers a use-after-free.
*/
wait_event(starget->sdev_wq, atomic_read(&starget->sdev_count) == 0);
} }
/** /**
...@@ -1645,6 +1656,9 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev) ...@@ -1645,6 +1656,9 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
list_add_tail(&sdev->same_target_siblings, &starget->devices); list_add_tail(&sdev->same_target_siblings, &starget->devices);
list_add_tail(&sdev->siblings, &shost->__devices); list_add_tail(&sdev->siblings, &shost->__devices);
spin_unlock_irqrestore(shost->host_lock, flags); spin_unlock_irqrestore(shost->host_lock, flags);
atomic_inc(&starget->sdev_count);
/* /*
* device can now only be removed via __scsi_remove_device() so hold * device can now only be removed via __scsi_remove_device() so hold
* the target. Target will be held in CREATED state until something * the target. Target will be held in CREATED state until something
......
...@@ -309,6 +309,8 @@ struct scsi_target { ...@@ -309,6 +309,8 @@ struct scsi_target {
struct list_head devices; struct list_head devices;
struct device dev; struct device dev;
struct kref reap_ref; /* last put renders target invisible */ struct kref reap_ref; /* last put renders target invisible */
atomic_t sdev_count;
wait_queue_head_t sdev_wq;
unsigned int channel; unsigned int channel;
unsigned int id; /* target id ... replace unsigned int id; /* target id ... replace
* scsi_device.id eventually */ * scsi_device.id eventually */
......
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