mirrors/scst
1
0
Fork 0
mirror of https://github.com/SCST-project/scst.git synced 2026-05-14 09:11:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix unloading of scst_vdisk while I/O is ongoing

Browse Source 
Call scst_free_device() directly instead of when the final dev->refcnt
put happens. Call scst_assign_dev_handler(dev, &scst_null_devtype) after
processing of I/O commands for 'dev' has finished. Hold the RCU read lock
while invoking percpu_ref_get(&dev->refcnt) in the LUN translation code.
This patch fixes the following crash when scst_vdisk is unloaded while I/O
is ongoing:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000368
IP: [<ffffffffc09a80b3>] blockio_exec+0x43/0x250 [scst_vdisk]
 [<ffffffffc14fea1b>] scst_do_real_exec+0x5b/0x240 [scst]
 [<ffffffffc150172f>] scst_exec_check_blocking+0x12f/0x250 [scst]
 [<ffffffffc1503791>] scst_process_active_cmd+0x91/0x1640 [scst]

Fixes: 3f2d50b589 ("scst: Do not suspend command processing when deleting a device")
Reported-by: Rongqing Tu <rongqing.tu@hpe.com>



git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@9048 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
Bart Van Assche
2020-07-12 23:35:03 +00:00
parent 34de8cd104
commit c5ff6adeae
6 changed files with 34 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
scst/include/scst.h
View File

@@ -2819,8 +2819,6 @@ struct scst_device {
/* Triggered when refcnt drops to zero. */
struct completion *remove_completion;
struct work_struct free_work;
/*
* Maximum count of uncompleted commands that an initiator could
* queue on this device. Then it will start getting TASK QUEUE FULL

3
scst/src/dev_handlers/scst_vdisk.c
View File

@@ -1321,8 +1321,6 @@ next:
if (res)
goto out;
percpu_ref_get(&dev->refcnt);
out:
TRACE_EXIT();
return res;
@@ -1347,7 +1345,6 @@ static void vdisk_detach(struct scst_device *dev)
/* virt_dev will be freed by the caller */
dev->dh_priv = NULL;
virt_dev->dev = NULL;
percpu_ref_put(&dev->refcnt);
TRACE_EXIT();
return;

7
scst/src/scst_lib.c
View File

@@ -4213,11 +4213,8 @@ static int scst_dif_none_type1(struct scst_cmd *cmd);
#endif
/* Called from thread context and hence may sleep. */
static void scst_free_device(struct work_struct *work)
void scst_free_device(struct scst_device *dev)
{
struct scst_device *dev = container_of(work, typeof(*dev),
free_work);
EXTRACHECKS_BUG_ON(dev->dev_scsi_atomic_cmd_active != 0);
EXTRACHECKS_BUG_ON(!list_empty(&dev->dev_exec_cmd_list));
@@ -4252,7 +4249,6 @@ static void scst_release_device(struct percpu_ref *ref)
if (c)
complete(c);
schedule_work(&dev->free_work);
}
int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev)
@@ -4273,7 +4269,6 @@ int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev)
memset(dev, 0, sizeof(*dev));
dev->handler = &scst_null_devtype;
INIT_WORK(&dev->free_work, scst_free_device);
res = percpu_ref_init(&dev->refcnt, scst_release_device, 0, GFP_KERNEL);
if (res < 0)
goto free_dev;

37
scst/src/scst_main.c
View File

@@ -1130,6 +1130,8 @@ out_del_unlocked:
out_free_dev:
percpu_ref_kill(&dev->refcnt);
scst_free_device(dev);
out_unlock:
mutex_unlock(&scst_mutex);
goto out;
@@ -1173,8 +1175,6 @@ static void scst_unregister_device(struct scsi_device *scsidp)
scst_dg_dev_remove_by_dev(dev);
scst_assign_dev_handler(dev, &scst_null_devtype);
list_for_each_entry_safe(acg_dev, aa, &dev->dev_acg_dev_list,
dev_acg_dev_list_entry) {
scst_acg_del_lun(acg_dev->acg, acg_dev->lun, true);
@@ -1184,8 +1184,6 @@ static void scst_unregister_device(struct scsi_device *scsidp)
mutex_unlock(&scst_mutex);
scst_dev_sysfs_del(dev);
PRINT_INFO("Detached from scsi%d, channel %d, id %d, lun %lld, type %d",
scsidp->host->host_no, scsidp->channel, scsidp->id,
(u64)scsidp->lun, scsidp->type);
@@ -1195,6 +1193,14 @@ static void scst_unregister_device(struct scsi_device *scsidp)
wait_for_completion(&c);
mutex_lock(&scst_mutex);
scst_assign_dev_handler(dev, &scst_null_devtype);
mutex_unlock(&scst_mutex);
scst_dev_sysfs_del(dev);
scst_free_device(dev);
out:
TRACE_EXIT();
return;
@@ -1399,6 +1405,7 @@ out_free_dev:
if (sysfs_del)
scst_dev_sysfs_del(dev);
percpu_ref_kill(&dev->refcnt);
scst_free_device(dev);
goto out;
out_unlock:
@@ -1444,8 +1451,6 @@ void scst_unregister_virtual_device(int id,
scst_dg_dev_remove_by_dev(dev);
scst_assign_dev_handler(dev, &scst_null_devtype);
list_for_each_entry_safe(acg_dev, aa, &dev->dev_acg_dev_list,
dev_acg_dev_list_entry) {
scst_acg_del_lun(acg_dev->acg, acg_dev->lun, true);
@@ -1455,8 +1460,6 @@ void scst_unregister_virtual_device(int id,
mutex_unlock(&scst_mutex);
scst_dev_sysfs_del(dev);
PRINT_INFO("Detached from virtual device %s (id %d)",
dev->virt_name, dev->virt_id);
@@ -1468,6 +1471,14 @@ void scst_unregister_virtual_device(int id,
wait_for_completion(&c);
mutex_lock(&scst_mutex);
scst_assign_dev_handler(dev, &scst_null_devtype);
mutex_unlock(&scst_mutex);
scst_dev_sysfs_del(dev);
scst_free_device(dev);
out:
TRACE_EXIT();
return;
@@ -1922,7 +1933,13 @@ out_err:
goto out;
}
/* The activity supposed to be suspended and scst_mutex held */
/*
* The caller must hold scst_mutex. No commands must be in progress for @dev.
* There are two ways to achieve this: either make sure no device handler is
* assigned before this function is called or remove all associated LUNs and
* wait until the commands associated with these LUNs have finished before this
* function is called.
*/
int scst_assign_dev_handler(struct scst_device *dev,
struct scst_dev_type *handler)
{
@@ -1932,6 +1949,8 @@ int scst_assign_dev_handler(struct scst_device *dev,
TRACE_ENTRY();
lockdep_assert_held(&scst_mutex);
sBUG_ON(handler == NULL);
if (dev->handler == handler)

1
scst/src/scst_priv.h
View File

@@ -370,6 +370,7 @@ int scst_alloc_tgt(struct scst_tgt_template *tgtt, struct scst_tgt **tgt);
void scst_free_tgt(struct scst_tgt *tgt);
int scst_alloc_device(gfp_t gfp_mask, int nodeid, struct scst_device **out_dev);
void scst_free_device(struct scst_device *dev);
bool scst_device_is_exported(struct scst_device *dev);
int scst_alloc_add_acg(struct scst_tgt *tgt, const char *acg_name,

7
scst/src/scst_targ.c
View File

@@ -3674,6 +3674,7 @@ static int scst_pre_xmit_response1(struct scst_cmd *cmd)
* latency, so we should decrement them after cmd completed.
*/
smp_mb__before_atomic_dec();
WARN_ON_ONCE(!cmd->owns_refcnt);
cmd->owns_refcnt = false;
atomic_dec(&cmd->tgt_dev->tgt_dev_cmd_count);
percpu_ref_put(&cmd->dev->refcnt);
@@ -4178,7 +4179,6 @@ static int scst_translate_lun(struct scst_cmd *cmd)
tgt_dev = scst_lookup_tgt_dev(cmd->sess, cmd->lun);
if (tgt_dev)
atomic_inc(&tgt_dev->tgt_dev_cmd_count);
rcu_read_unlock();
if (tgt_dev) {
struct scst_device *dev = tgt_dev->dev;
@@ -4191,6 +4191,8 @@ static int scst_translate_lun(struct scst_cmd *cmd)
cmd->cur_order_data = tgt_dev->curr_order_data;
cmd->dev = dev;
cmd->devt = dev->handler;
cmd->owns_refcnt = true;
percpu_ref_get(&dev->refcnt);
res = 0;
} else {
@@ -4201,6 +4203,7 @@ static int scst_translate_lun(struct scst_cmd *cmd)
atomic_dec(&tgt_dev->tgt_dev_cmd_count);
}
}
rcu_read_unlock();
if (unlikely(res != 0)) {
if (!nul_dev) {
TRACE(TRACE_MINOR,
@@ -4340,8 +4343,6 @@ static int __scst_init_cmd(struct scst_cmd *cmd)
failure = true;
}
cmd->owns_refcnt = true;
percpu_ref_get(&dev->refcnt);
#ifdef CONFIG_SCST_PER_DEVICE_CMD_COUNT_LIMIT
atomic_inc(&dev->dev_cmd_count);
cnt = atomic_read(&dev->dev_cmd_count);