From b7991306809cc538b691a2e9d34dcede9dce9648 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 14 May 2017 14:55:00 +0000 Subject: [PATCH 1/4] ib_srpt: Fix a logging statement git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7182 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- srpt/src/ib_srpt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srpt/src/ib_srpt.c b/srpt/src/ib_srpt.c index 5116b8278..c65c76b15 100644 --- a/srpt/src/ib_srpt.c +++ b/srpt/src/ib_srpt.c @@ -4663,7 +4663,7 @@ static int __init srpt_init_module(void) if (srpt_sq_size < MIN_SRPT_SQ_SIZE) { pr_err("invalid value %d for kernel module parameter srpt_sq_size -- must be at least %d.\n", - srpt_srq_size, MIN_SRPT_SQ_SIZE); + srpt_sq_size, MIN_SRPT_SQ_SIZE); goto out; } From cda5cadb1c449f8e00b2d7179abc8f149873b70b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 14 May 2017 18:49:04 +0000 Subject: [PATCH 2/4] isert-scst: Enable type checking for isert portal pointers This patch does not change any functionality. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7183 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser_datamover.c | 6 ++---- iscsi-scst/kernel/isert-scst/iser_datamover.h | 4 ++-- iscsi-scst/kernel/isert-scst/isert.h | 3 ++- iscsi-scst/kernel/isert-scst/isert_login.c | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/iscsi-scst/kernel/isert-scst/iser_datamover.c b/iscsi-scst/kernel/isert-scst/iser_datamover.c index 2ea4ff5ba..eca083529 100644 --- a/iscsi-scst/kernel/isert-scst/iser_datamover.c +++ b/iscsi-scst/kernel/isert-scst/iser_datamover.c @@ -93,15 +93,13 @@ out: return ret; } -void *isert_portal_add(struct sockaddr *saddr, size_t addr_len) +struct isert_portal *isert_portal_add(struct sockaddr *saddr, size_t addr_len) { return isert_portal_start(saddr, addr_len); } -int isert_portal_remove(void *portal_h) +int isert_portal_remove(struct isert_portal *portal) { - struct isert_portal *portal = portal_h; - isert_portal_release(portal); return 0; } diff --git a/iscsi-scst/kernel/isert-scst/iser_datamover.h b/iscsi-scst/kernel/isert-scst/iser_datamover.h index 8b749bdfa..ec1c8a378 100644 --- a/iscsi-scst/kernel/isert-scst/iser_datamover.h +++ b/iscsi-scst/kernel/isert-scst/iser_datamover.h @@ -42,8 +42,8 @@ int isert_datamover_init(void); int isert_datamover_cleanup(void); -void *isert_portal_add(struct sockaddr *sa, size_t addr_len); -int isert_portal_remove(void *portal_h); +struct isert_portal *isert_portal_add(struct sockaddr *sa, size_t addr_len); +int isert_portal_remove(struct isert_portal *portal); struct iscsi_cmnd *isert_alloc_login_rsp_pdu(struct iscsi_conn *iscsi_conn); diff --git a/iscsi-scst/kernel/isert-scst/isert.h b/iscsi-scst/kernel/isert-scst/isert.h index 7a4554bce..dd541a1b2 100644 --- a/iscsi-scst/kernel/isert-scst/isert.h +++ b/iscsi-scst/kernel/isert-scst/isert.h @@ -63,6 +63,7 @@ #include "iser_hdr.h" struct iscsi_conn; +struct isert_portal; #define ISERT_NR_DEVS 128 @@ -76,7 +77,7 @@ struct isert_listener_dev { struct list_head curr_conn_list; struct isert_addr_info info; atomic_t available; - void *portal_h[ISERT_MAX_PORTALS]; + struct isert_portal *portal_h[ISERT_MAX_PORTALS]; int free_portal_idx; }; diff --git a/iscsi-scst/kernel/isert-scst/isert_login.c b/iscsi-scst/kernel/isert-scst/isert_login.c index 6c9e2eac4..99ed1a909 100644 --- a/iscsi-scst/kernel/isert-scst/isert_login.c +++ b/iscsi-scst/kernel/isert-scst/isert_login.c @@ -395,7 +395,7 @@ static long isert_listen_ioctl(struct file *filp, unsigned int cmd, struct isert_listener_dev *dev = filp->private_data; int res = 0, rc; void __user *ptr = (void __user *)arg; - void *portal; + struct isert_portal *portal; TRACE_ENTRY(); From 830369e50d72eef40ca17266b0c16b6bc419895f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 14 May 2017 18:53:01 +0000 Subject: [PATCH 3/4] isert-scst: Avoid that shutdown sporadically hangs Waiting for other threads to release an object using code like "while (object->refcnt > 0) msleep(100)" without holding a reference on 'object' is wrong because the memory object points at may be freed before or while this loop is in progress. Hence introduce a global portal object count and wait on that count instead of waiting for the per-portal reference count to reach zero. The use-after-free was introduced in r6952 ("isert: faster release of isert_scst module"). See also https://sourceforge.net/p/scst/tickets/2/. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7184 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/isert-scst/iser.h | 5 ++++ iscsi-scst/kernel/isert-scst/iser_global.c | 31 ++++++++++++++++++++++ iscsi-scst/kernel/isert-scst/iser_rdma.c | 5 ++-- iscsi-scst/kernel/isert-scst/isert_login.c | 1 + 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/iscsi-scst/kernel/isert-scst/iser.h b/iscsi-scst/kernel/isert-scst/iser.h index 65ae7e3db..a638c296f 100644 --- a/iscsi-scst/kernel/isert-scst/iser.h +++ b/iscsi-scst/kernel/isert-scst/iser.h @@ -256,6 +256,9 @@ struct isert_global { spinlock_t portal_lock; /* protected by portal_lock */ struct list_head portal_list; + /* Number of live portal objects. Protected by portal_lock. */ + int portal_cnt; + wait_queue_head_t portal_wq; /* protected by dev_list_mutex */ struct list_head dev_list; struct workqueue_struct *conn_wq; @@ -270,6 +273,8 @@ int isert_datamover_cleanup(void); void isert_portal_list_add(struct isert_portal *portal); void isert_portal_list_remove(struct isert_portal *portal); +void isert_decrease_portal_cnt(void); +void isert_wait_for_portal_release(void); void isert_dev_list_add(struct isert_device *isert_dev); void isert_dev_list_remove(struct isert_device *isert_dev); diff --git a/iscsi-scst/kernel/isert-scst/iser_global.c b/iscsi-scst/kernel/isert-scst/iser_global.c index 9834fc481..82b43cbcb 100644 --- a/iscsi-scst/kernel/isert-scst/iser_global.c +++ b/iscsi-scst/kernel/isert-scst/iser_global.c @@ -49,6 +49,7 @@ void isert_portal_list_add(struct isert_portal *portal) { spin_lock(&isert_glob.portal_lock); list_add_tail(&portal->list_node, &isert_glob.portal_list); + isert_glob.portal_cnt++; spin_unlock(&isert_glob.portal_lock); } @@ -59,6 +60,32 @@ void isert_portal_list_remove(struct isert_portal *portal) spin_unlock(&isert_glob.portal_lock); } +void isert_decrease_portal_cnt(void) +{ + spin_lock(&isert_glob.portal_lock); + WARN_ON_ONCE(isert_glob.portal_cnt <= 0); + spin_unlock(&isert_glob.portal_lock); + + if (--isert_glob.portal_cnt == 0) + wake_up_all(&isert_glob.portal_wq); +} + +static int isert_portal_cnt(void) +{ + int portal_cnt; + + spin_lock(&isert_glob.portal_lock); + portal_cnt = isert_glob.portal_cnt; + spin_unlock(&isert_glob.portal_lock); + + return portal_cnt; +} + +void isert_wait_for_portal_release(void) +{ + wait_event(isert_glob.portal_wq, isert_portal_cnt() == 0); +} + void isert_dev_list_add(struct isert_device *isert_dev) { list_add_tail(&isert_dev->devs_node, &isert_glob.dev_list); @@ -90,6 +117,7 @@ void isert_portal_list_release_all(void) list_for_each_entry_safe(portal, n, &isert_glob.portal_list, list_node) isert_portal_release(portal); + isert_wait_for_portal_release(); } void isert_conn_queue_work(struct work_struct *w) @@ -99,10 +127,13 @@ void isert_conn_queue_work(struct work_struct *w) int isert_global_init(void) { + isert_glob.portal_cnt = 0; + INIT_LIST_HEAD(&isert_glob.portal_list); INIT_LIST_HEAD(&isert_glob.dev_list); spin_lock_init(&isert_glob.portal_lock); + init_waitqueue_head(&isert_glob.portal_wq); isert_glob.conn_wq = create_workqueue("isert_conn_wq"); if (!isert_glob.conn_wq) { diff --git a/iscsi-scst/kernel/isert-scst/iser_rdma.c b/iscsi-scst/kernel/isert-scst/iser_rdma.c index f38207573..10051f7cd 100644 --- a/iscsi-scst/kernel/isert-scst/iser_rdma.c +++ b/iscsi-scst/kernel/isert-scst/iser_rdma.c @@ -1900,6 +1900,8 @@ static void isert_portal_free(struct isert_portal *portal) kfree(portal); module_put(THIS_MODULE); + + isert_decrease_portal_cnt(); } void isert_portal_release(struct isert_portal *portal) @@ -1925,9 +1927,6 @@ void isert_portal_release(struct isert_portal *portal) isert_portal_free(portal); mutex_unlock(&dev_list_mutex); - while (portal->refcnt > 0) - msleep(100); - PRINT_INFO("done releasing portal %p", portal); } diff --git a/iscsi-scst/kernel/isert-scst/isert_login.c b/iscsi-scst/kernel/isert-scst/isert_login.c index 99ed1a909..d1ed53f07 100644 --- a/iscsi-scst/kernel/isert-scst/isert_login.c +++ b/iscsi-scst/kernel/isert-scst/isert_login.c @@ -1052,6 +1052,7 @@ void isert_close_all_portals(void) for (i = 0; i < isert_listen_dev.free_portal_idx; ++i) isert_portal_remove(isert_listen_dev.portal_h[i]); + isert_wait_for_portal_release(); isert_listen_dev.free_portal_idx = 0; } From b2b15e1d55288b5bd3fde89162f2ea70c77d920a Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 14 May 2017 18:56:09 +0000 Subject: [PATCH 4/4] scst_local: Fix a race condition Avoid that the following crash can occur: general protection fault: 0000 [#1] PREEMPT SMP RIP: 0010:scsi_is_host_device+0x7/0x20 [scsi_mod] Call Trace: scst_process_aens+0x95/0x1d0 [scst_local] scst_aen_work_fn+0x6f/0x120 [scst_local] process_one_work+0x20b/0x6c0 worker_thread+0x4e/0x4a0 kthread+0x113/0x150 ret_from_fork+0x31/0x40 git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7185 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst_local/scst_local.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/scst_local/scst_local.c b/scst_local/scst_local.c index d7c451f3d..e5d91d876 100644 --- a/scst_local/scst_local.c +++ b/scst_local/scst_local.c @@ -1265,6 +1265,7 @@ static void scst_process_aens(struct scst_local_sess *sess, __acquires(&sess->aen_lock) { struct scst_aen_work_item *work_item = NULL; + struct Scsi_Host *shost; TRACE_ENTRY(); @@ -1274,7 +1275,9 @@ static void scst_process_aens(struct scst_local_sess *sess, work_item = list_first_entry(&sess->aen_work_list, struct scst_aen_work_item, work_list_entry); list_del(&work_item->work_list_entry); - + shost = sess->shost; + if (shost && !scsi_host_get(shost)) + shost = NULL; spin_unlock(&sess->aen_lock); if (cleanup_only) @@ -1283,13 +1286,17 @@ static void scst_process_aens(struct scst_local_sess *sess, sBUG_ON(work_item->aen->event_fn != SCST_AEN_SCSI); /* Let's always rescan */ - scsi_scan_target(&sess->shost->shost_gendev, 0, 0, - SCAN_WILD_CARD, 1); + if (shost) + scsi_scan_target(&shost->shost_gendev, 0, 0, + SCAN_WILD_CARD, 1); done: scst_aen_done(work_item->aen); kfree(work_item); + if (shost) + scsi_host_put(shost); + spin_lock(&sess->aen_lock); } @@ -1707,12 +1714,18 @@ out: static int scst_local_driver_remove(struct device *dev) { struct scst_local_sess *sess; + struct Scsi_Host *shost = NULL; TRACE_ENTRY(); sess = to_scst_lcl_sess(dev); - scsi_remove_host(sess->shost); - scsi_host_put(sess->shost); + + spin_lock(&sess->aen_lock); + swap(sess->shost, shost); + spin_unlock(&sess->aen_lock); + + scsi_remove_host(shost); + scsi_host_put(shost); TRACE_EXIT(); return 0;