[SCSI] target: fix use after free detected by SLUB poison

This patch moves a large number of memory release paths inside of the
configfs callback target_core_hba_item_ops->release() called from
within fs/configfs/item.c: config_item_cleanup() context.  This patch
resolves the SLUB 'Poison overwritten' warnings.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>

1 files changed, 74 insertions, 46 deletions

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index e77001b16063..caf8dc18ee0a 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -1970,13 +1970,35 @@ static void target_core_dev_release(struct config_item *item)
 {
         struct se_subsystem_dev *se_dev = container_of(to_config_group(item),
                                 struct se_subsystem_dev, se_dev_group);
-        struct config_group *dev_cg;
-
-        if (!(se_dev))
-                return;
+        struct se_hba *hba = item_to_hba(&se_dev->se_dev_hba->hba_group.cg_item);
+        struct se_subsystem_api *t = hba->transport;
+        struct config_group *dev_cg = &se_dev->se_dev_group;
 
-        dev_cg = &se_dev->se_dev_group;
         kfree(dev_cg->default_groups);
+        /*
+         * This pointer will set when the storage is enabled with:
+         *`echo 1 > $CONFIGFS/core/$HBA/$DEV/dev_enable`
+         */
+        if (se_dev->se_dev_ptr) {
+                printk(KERN_INFO "Target_Core_ConfigFS: Calling se_free_"
+                        "virtual_device() for se_dev_ptr: %p\n",
+                        se_dev->se_dev_ptr);
+
+                se_free_virtual_device(se_dev->se_dev_ptr, hba);
+        } else {
+                /*
+                 * Release struct se_subsystem_dev->se_dev_su_ptr..
+                 */
+                printk(KERN_INFO "Target_Core_ConfigFS: Calling t->free_"
+                        "device() for se_dev_su_ptr: %p\n",
+                        se_dev->se_dev_su_ptr);
+
+                t->free_device(se_dev->se_dev_su_ptr);
+        }
+
+        printk(KERN_INFO "Target_Core_ConfigFS: Deallocating se_subsystem"
+                        "_dev_t: %p\n", se_dev);
+        kfree(se_dev);
 }
 
 static ssize_t target_core_dev_show(struct config_item *item,
@@ -2139,7 +2161,16 @@ static struct configfs_attribute *target_core_alua_lu_gp_attrs[] = {
         NULL,
 };
 
+static void target_core_alua_lu_gp_release(struct config_item *item)
+{
+        struct t10_alua_lu_gp *lu_gp = container_of(to_config_group(item),
+                        struct t10_alua_lu_gp, lu_gp_group);
+
+        core_alua_free_lu_gp(lu_gp);
+}
+
 static struct configfs_item_operations target_core_alua_lu_gp_ops = {
+        .release                = target_core_alua_lu_gp_release,
         .show_attribute         = target_core_alua_lu_gp_attr_show,
         .store_attribute        = target_core_alua_lu_gp_attr_store,
 };
@@ -2190,9 +2221,11 @@ static void target_core_alua_drop_lu_gp(
         printk(KERN_INFO "Target_Core_ConfigFS: Releasing ALUA Logical Unit"
                 " Group: core/alua/lu_gps/%s, ID: %hu\n",
                 config_item_name(item), lu_gp->lu_gp_id);
-
+        /*
+         * core_alua_free_lu_gp() is called from target_core_alua_lu_gp_ops->release()
+         * -> target_core_alua_lu_gp_release()
+         */
         config_item_put(item);
-        core_alua_free_lu_gp(lu_gp);
 }
 
 static struct configfs_group_operations target_core_alua_lu_gps_group_ops = {
@@ -2548,7 +2581,16 @@ static struct configfs_attribute *target_core_alua_tg_pt_gp_attrs[] = {
         NULL,
 };
 
+static void target_core_alua_tg_pt_gp_release(struct config_item *item)
+{
+        struct t10_alua_tg_pt_gp *tg_pt_gp = container_of(to_config_group(item),
+                        struct t10_alua_tg_pt_gp, tg_pt_gp_group);
+
+        core_alua_free_tg_pt_gp(tg_pt_gp);
+}
+
 static struct configfs_item_operations target_core_alua_tg_pt_gp_ops = {
+        .release                = target_core_alua_tg_pt_gp_release,
         .show_attribute         = target_core_alua_tg_pt_gp_attr_show,
         .store_attribute        = target_core_alua_tg_pt_gp_attr_store,
 };
@@ -2601,9 +2643,11 @@ static void target_core_alua_drop_tg_pt_gp(
         printk(KERN_INFO "Target_Core_ConfigFS: Releasing ALUA Target Port"
                 " Group: alua/tg_pt_gps/%s, ID: %hu\n",
                 config_item_name(item), tg_pt_gp->tg_pt_gp_id);
-
+        /*
+         * core_alua_free_tg_pt_gp() is called from target_core_alua_tg_pt_gp_ops->release()
+         * -> target_core_alua_tg_pt_gp_release().
+         */
         config_item_put(item);
-        core_alua_free_tg_pt_gp(tg_pt_gp);
 }
 
 static struct configfs_group_operations target_core_alua_tg_pt_gps_group_ops = {
@@ -2770,13 +2814,11 @@ static void target_core_drop_subdev(
         struct se_subsystem_api *t;
         struct config_item *df_item;
         struct config_group *dev_cg, *tg_pt_gp_cg;
-        int i, ret;
+        int i;
 
         hba = item_to_hba(&se_dev->se_dev_hba->hba_group.cg_item);
 
-        if (mutex_lock_interruptible(&hba->hba_access_mutex))
-                goto out;
-
+        mutex_lock(&hba->hba_access_mutex);
         t = hba->transport;
 
         spin_lock(&se_global->g_device_lock);
@@ -2790,7 +2832,10 @@ static void target_core_drop_subdev(
                 config_item_put(df_item);
         }
         kfree(tg_pt_gp_cg->default_groups);
-        core_alua_free_tg_pt_gp(T10_ALUA(se_dev)->default_tg_pt_gp);
+        /*
+         * core_alua_free_tg_pt_gp() is called from ->default_tg_pt_gp
+         * directly from target_core_alua_tg_pt_gp_release().
+         */
         T10_ALUA(se_dev)->default_tg_pt_gp = NULL;
 
         dev_cg = &se_dev->se_dev_group;
@@ -2799,38 +2844,12 @@ static void target_core_drop_subdev(
                 dev_cg->default_groups[i] = NULL;
                 config_item_put(df_item);
         }
-
-        config_item_put(item);
         /*
-         * This pointer will set when the storage is enabled with:
-         * `echo 1 > $CONFIGFS/core/$HBA/$DEV/dev_enable`
+         * The releasing of se_dev and associated se_dev->se_dev_ptr is done
+         * from target_core_dev_item_ops->release() ->target_core_dev_release().
          */
-        if (se_dev->se_dev_ptr) {
-                printk(KERN_INFO "Target_Core_ConfigFS: Calling se_free_"
-                        "virtual_device() for se_dev_ptr: %p\n",
-                                se_dev->se_dev_ptr);
-
-                ret = se_free_virtual_device(se_dev->se_dev_ptr, hba);
-                if (ret < 0)
-                        goto hba_out;
-        } else {
-                /*
-                 * Release struct se_subsystem_dev->se_dev_su_ptr..
-                 */
-                printk(KERN_INFO "Target_Core_ConfigFS: Calling t->free_"
-                        "device() for se_dev_su_ptr: %p\n",
-                        se_dev->se_dev_su_ptr);
-
-                t->free_device(se_dev->se_dev_su_ptr);
-        }
-
-        printk(KERN_INFO "Target_Core_ConfigFS: Deallocating se_subsystem"
-                "_dev_t: %p\n", se_dev);
-
-hba_out:
+        config_item_put(item);
         mutex_unlock(&hba->hba_access_mutex);
-out:
-        kfree(se_dev);
 }
 
 static struct configfs_group_operations target_core_hba_group_ops = {
@@ -2913,6 +2932,13 @@ SE_HBA_ATTR(hba_mode, S_IRUGO | S_IWUSR);
 
 CONFIGFS_EATTR_OPS(target_core_hba, se_hba, hba_group);
 
+static void target_core_hba_release(struct config_item *item)
+{
+        struct se_hba *hba = container_of(to_config_group(item),
+                                struct se_hba, hba_group);
+        core_delete_hba(hba);
+}
+
 static struct configfs_attribute *target_core_hba_attrs[] = {
         &target_core_hba_hba_info.attr,
         &target_core_hba_hba_mode.attr,
@@ -2920,6 +2946,7 @@ static struct configfs_attribute *target_core_hba_attrs[] = {
 };
 
 static struct configfs_item_operations target_core_hba_item_ops = {
+        .release                = target_core_hba_release,
         .show_attribute         = target_core_hba_attr_show,
         .store_attribute        = target_core_hba_attr_store,
 };
@@ -2996,10 +3023,11 @@ static void target_core_call_delhbafromtarget(
         struct config_group *group,
         struct config_item *item)
 {
-        struct se_hba *hba = item_to_hba(item);
-
+        /*
+         * core_delete_hba() is called from target_core_hba_item_ops->release()
+         * -> target_core_hba_release()
+         */
         config_item_put(item);
-        core_delete_hba(hba);
 }
 
 static struct configfs_group_operations target_core_group_ops = {