From 36f2612eb1f0a68062d28c3ba5fae99eaed5790f Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 20 May 2017 03:00:28 +0000 Subject: [PATCH 1/3] scst: fix possible NULL dereference in TM code TM command accessing a non-existing LUN might lead NULL dereference in scst_call_dev_task_mgmt_fn_done(). Reported-By: > git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7190 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_targ.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 2db1d5950..b6d53b6cd 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -7393,7 +7393,8 @@ static int scst_mgmt_affected_cmds_done(struct scst_mgmt_cmd *mcmd) case SCST_CLEAR_TASK_SET: case SCST_PR_ABORT_ALL: case SCST_LUN_RESET: - scst_call_dev_task_mgmt_fn_done(mcmd, mcmd->mcmd_tgt_dev); + if (mcmd->mcmd_tgt_dev != NULL) + scst_call_dev_task_mgmt_fn_done(mcmd, mcmd->mcmd_tgt_dev); break; case SCST_TARGET_RESET: From e452378d1e9e9f8d11aa3b79c4707d33f41ad716 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 20 May 2017 03:19:54 +0000 Subject: [PATCH 2/3] scst: restore locking documentation comments Better to follow the common rule and always have locking expectations documented in addition to runtime checks. Partial revert of r7095. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7191 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_copy_mgr.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scst/src/scst_copy_mgr.c b/scst/src/scst_copy_mgr.c index d7206f43d..5661c2b0d 100644 --- a/scst/src/scst_copy_mgr.c +++ b/scst/src/scst_copy_mgr.c @@ -2498,6 +2498,7 @@ out_free: goto out; } +/* scst_mutex supposed to be held and activities suspended */ static bool scst_cm_is_lun_free(unsigned int lun) { bool res = true; @@ -2520,6 +2521,7 @@ static bool scst_cm_is_lun_free(unsigned int lun) return res; } +/* scst_mutex supposed to be held and activities suspended */ static unsigned int scst_cm_get_lun(const struct scst_device *dev) { unsigned int res = SCST_MAX_LUN; @@ -2549,6 +2551,7 @@ out: return res; } +/* scst_mutex supposed to be held and activities suspended */ static int scst_cm_dev_register(struct scst_device *dev, uint64_t lun) { int res, i; @@ -2624,6 +2627,7 @@ out_err: goto out; } +/* scst_mutex supposed to be held and activities suspended */ static void scst_cm_dev_unregister(struct scst_device *dev, bool del_lun) { int i; @@ -2700,6 +2704,7 @@ out_unblock: goto out_resume; } +/* scst_mutex supposed to be held and activities suspended */ int scst_cm_on_dev_register(struct scst_device *dev) { int res = 0; @@ -2719,6 +2724,7 @@ out: return res; } +/* scst_mutex supposed to be held and activities suspended */ void scst_cm_on_dev_unregister(struct scst_device *dev) { TRACE_ENTRY(); @@ -2732,6 +2738,7 @@ void scst_cm_on_dev_unregister(struct scst_device *dev) return; } +/* scst_mutex supposed to be held and activities suspended */ int scst_cm_on_add_acg(struct scst_acg *acg) { int res = 0; @@ -2757,6 +2764,7 @@ out: return res; } +/* scst_mutex supposed to be held and activities suspended */ void scst_cm_on_del_acg(struct scst_acg *acg) { scst_assert_activity_suspended(); @@ -2764,6 +2772,7 @@ void scst_cm_on_del_acg(struct scst_acg *acg) /* Nothing to do */ } +/* scst_mutex supposed to be held and activities suspended */ int scst_cm_on_add_lun(struct scst_acg_dev *acg_dev, uint64_t lun, unsigned int *flags) { @@ -2791,6 +2800,7 @@ out: return res; } +/* scst_mutex supposed to be held and activities suspended */ bool scst_cm_on_del_lun(struct scst_acg_dev *acg_dev, bool gen_report_luns_changed) { bool res = gen_report_luns_changed; @@ -2812,6 +2822,7 @@ out: return res; } +/* scst_mutex2 supposed to be held */ static bool scst_cm_check_access_acg(const char *initiator_name, const struct scst_device *dev, const struct scst_acg *acg, bool default_acg) From c729d1934ad91ed43a61fa2ea72f3376088ebdc8 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 20 May 2017 03:29:41 +0000 Subject: [PATCH 3/3] scst: minor cleanup Let's keep it simple without unused functionality git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7192 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_main.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index 1440bc4b1..30ce3954e 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -1270,10 +1270,7 @@ static struct scst_device *__scst_lookup_device(struct scsi_device *scsidp) return NULL; } -static void scst_unregister_device(struct scsi_device *scsidp, - void (*on_free)(struct scst_device *dev, - void *arg), - void *arg) +static void scst_unregister_device(struct scsi_device *scsidp) { struct scst_device *dev; struct scst_acg_dev *acg_dev, *aa; @@ -1331,9 +1328,6 @@ static void scst_unregister_device(struct scsi_device *scsidp, scsidp->host->host_no, scsidp->channel, scsidp->id, (u64)scsidp->lun, scsidp->type); - if (on_free) - on_free(dev, arg); - scst_free_device(dev); out: @@ -2420,7 +2414,7 @@ static void scst_remove(struct device *cdev, struct class_interface *intf) if ((scsidp->host->hostt->name == NULL) || (strcmp(scsidp->host->hostt->name, SCST_LOCAL_NAME) != 0)) - scst_unregister_device(scsidp, NULL, NULL); + scst_unregister_device(scsidp); TRACE_EXIT(); return;