From 8fdd45a8299c3eda3c552fcbb7599484b4eb0175 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 14 Sep 2015 03:14:06 +0000 Subject: [PATCH] scst_lib: Fix a stack overflow Avoid that removing an acg with which a large number of sessions has been associated triggers a stack overflow due to the following recursive call chain: scst_put_acg() -> scst_release_acg() -> scst_free_acg() -> scst_free_acn() -> scst_check_reassign_sessions() -> scst_check_reassign_sess() -> scst_put_acg(). Signed-off-by: Bart Van Assche git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@6500 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 2 ++ scst/src/scst_lib.c | 30 ++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 0d19a44c3..ac581a097 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -3307,6 +3307,8 @@ struct scst_acg { /* One more than the number of sessions in acg_sess_list */ struct kref acg_kref; + struct work_struct put_work; + /* Owner target */ struct scst_tgt *tgt; diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 6a8bef0e3..b81559787 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -61,6 +61,7 @@ #include "scst_mem.h" #include "scst_pres.h" +static void scst_put_acg_work(struct work_struct *work); static void scst_del_acn(struct scst_acn *acn); static void scst_free_acn(struct scst_acn *acn, bool reassign); @@ -72,6 +73,7 @@ struct scsi_io_context { }; static struct kmem_cache *scsi_io_context_cache; #endif +static struct workqueue_struct *scst_release_acg_wq; #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 22) \ && (!defined(RHEL_RELEASE_CODE) || RHEL_RELEASE_CODE -0 < 5 * 256 + 3) \ @@ -4292,6 +4294,7 @@ struct scst_acg *scst_alloc_add_acg(struct scst_tgt *tgt, } kref_init(&acg->acg_kref); + INIT_WORK(&acg->put_work, scst_put_acg_work); acg->tgt = tgt; INIT_LIST_HEAD(&acg->acg_dev_list); INIT_LIST_HEAD(&acg->acg_sess_list); @@ -4328,6 +4331,8 @@ struct scst_acg *scst_alloc_add_acg(struct scst_tgt *tgt, } #endif + kobject_get(&tgt->tgt_kobj); + out: TRACE_EXIT_HRES(acg); return acg; @@ -4389,8 +4394,9 @@ static void scst_free_acg(struct scst_acg *acg) { struct scst_acg_dev *acg_dev, *acg_dev_tmp; struct scst_acn *acn, *acnt; + struct scst_tgt *tgt = acg->tgt; - TRACE_DBG("Freeing acg %s/%s", acg->tgt->tgt_name, acg->acg_name); + TRACE_DBG("Freeing acg %s/%s", tgt->tgt_name, acg->acg_name); list_for_each_entry_safe(acg_dev, acg_dev_tmp, &acg->acg_dev_list, acg_dev_list_entry) { @@ -4411,6 +4417,8 @@ static void scst_free_acg(struct scst_acg *acg) kfree(acg->acg_name); kfree(acg); + + kobject_put(&tgt->tgt_kobj); } static void scst_release_acg(struct kref *kref) @@ -4420,9 +4428,20 @@ static void scst_release_acg(struct kref *kref) scst_free_acg(acg); } +static void scst_put_acg_work(struct work_struct *work) +{ + struct scst_acg *acg = container_of(work, typeof(*acg), put_work); + + kref_put(&acg->acg_kref, scst_release_acg); +} + void scst_put_acg(struct scst_acg *acg) { - kref_put(&acg->acg_kref, scst_release_acg); + /* + * Schedule the kref_put() call instead of invoking it directly to + * avoid deep recursion and a stack overflow. + */ + queue_work(scst_release_acg_wq, &acg->put_work); } void scst_get_acg(struct scst_acg *acg) @@ -13503,6 +13522,9 @@ static void __init scst_scsi_op_list_init(void) TRACE_BUFFER("scst_scsi_op_list", scst_scsi_op_list, sizeof(scst_scsi_op_list)); + scst_release_acg_wq = create_workqueue("scst_release_acg"); + WARN_ON_ONCE(IS_ERR(scst_release_acg_wq)); + TRACE_EXIT(); return; } @@ -13532,6 +13554,10 @@ out: void scst_lib_exit(void) { + /* Wait until any ongoing acg->put_work has finished. */ + flush_workqueue(scst_release_acg_wq); + destroy_workqueue(scst_release_acg_wq); + #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 30) BUILD_BUG_ON(SCST_MAX_CDB_SIZE != BLK_MAX_CDB); BUILD_BUG_ON(SCST_SENSE_BUFFERSIZE < SCSI_SENSE_BUFFERSIZE);