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
This commit is contained in:
Bart Van Assche
2017-05-14 18:53:01 +00:00
parent cda5cadb1c
commit 830369e50d
4 changed files with 39 additions and 3 deletions

View File

@@ -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);

View File

@@ -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) {

View File

@@ -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);
}

View File

@@ -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;
}