From 1f6fe7cba1c0a817a8712d7fdd0ec1b4ddd4ea2f Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 9 Feb 2011 15:34:54 -0800 Subject: [PATCH] [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 Signed-off-by: James Bottomley --- drivers/target/target_core_configfs.c | 120 ++++++++++++------- drivers/target/target_core_fabric_configfs.c | 92 +++++++++----- include/target/target_core_base.h | 1 + 3 files changed, 140 insertions(+), 73 deletions(-) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index e77001b1606..caf8dc18ee0 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 = { diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 32b148d7e26..b65d1c8e774 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -214,12 +214,22 @@ TCM_MAPPEDLUN_ATTR(write_protect, S_IRUGO | S_IWUSR); CONFIGFS_EATTR_OPS(target_fabric_mappedlun, se_lun_acl, se_lun_group); +static void target_fabric_mappedlun_release(struct config_item *item) +{ + struct se_lun_acl *lacl = container_of(to_config_group(item), + struct se_lun_acl, se_lun_group); + struct se_portal_group *se_tpg = lacl->se_lun_nacl->se_tpg; + + core_dev_free_initiator_node_lun_acl(se_tpg, lacl); +} + static struct configfs_attribute *target_fabric_mappedlun_attrs[] = { &target_fabric_mappedlun_write_protect.attr, NULL, }; static struct configfs_item_operations target_fabric_mappedlun_item_ops = { + .release = target_fabric_mappedlun_release, .show_attribute = target_fabric_mappedlun_attr_show, .store_attribute = target_fabric_mappedlun_attr_store, .allow_link = target_fabric_mappedlun_link, @@ -337,15 +347,21 @@ static void target_fabric_drop_mappedlun( struct config_group *group, struct config_item *item) { - struct se_lun_acl *lacl = container_of(to_config_group(item), - struct se_lun_acl, se_lun_group); - struct se_portal_group *se_tpg = lacl->se_lun_nacl->se_tpg; - config_item_put(item); - core_dev_free_initiator_node_lun_acl(se_tpg, lacl); +} + +static void target_fabric_nacl_base_release(struct config_item *item) +{ + struct se_node_acl *se_nacl = container_of(to_config_group(item), + struct se_node_acl, acl_group); + struct se_portal_group *se_tpg = se_nacl->se_tpg; + struct target_fabric_configfs *tf = se_tpg->se_tpg_wwn->wwn_tf; + + tf->tf_ops.fabric_drop_nodeacl(se_nacl); } static struct configfs_item_operations target_fabric_nacl_base_item_ops = { + .release = target_fabric_nacl_base_release, .show_attribute = target_fabric_nacl_base_attr_show, .store_attribute = target_fabric_nacl_base_attr_store, }; @@ -404,9 +420,6 @@ static void target_fabric_drop_nodeacl( struct config_group *group, struct config_item *item) { - struct se_portal_group *se_tpg = container_of(group, - struct se_portal_group, tpg_acl_group); - struct target_fabric_configfs *tf = se_tpg->se_tpg_wwn->wwn_tf; struct se_node_acl *se_nacl = container_of(to_config_group(item), struct se_node_acl, acl_group); struct config_item *df_item; @@ -419,9 +432,10 @@ static void target_fabric_drop_nodeacl( nacl_cg->default_groups[i] = NULL; config_item_put(df_item); } - + /* + * struct se_node_acl free is done in target_fabric_nacl_base_release() + */ config_item_put(item); - tf->tf_ops.fabric_drop_nodeacl(se_nacl); } static struct configfs_group_operations target_fabric_nacl_group_ops = { @@ -437,7 +451,18 @@ TF_CIT_SETUP(tpg_nacl, NULL, &target_fabric_nacl_group_ops, NULL); CONFIGFS_EATTR_OPS(target_fabric_np_base, se_tpg_np, tpg_np_group); +static void target_fabric_np_base_release(struct config_item *item) +{ + struct se_tpg_np *se_tpg_np = container_of(to_config_group(item), + struct se_tpg_np, tpg_np_group); + struct se_portal_group *se_tpg = se_tpg_np->tpg_np_parent; + struct target_fabric_configfs *tf = se_tpg->se_tpg_wwn->wwn_tf; + + tf->tf_ops.fabric_drop_np(se_tpg_np); +} + static struct configfs_item_operations target_fabric_np_base_item_ops = { + .release = target_fabric_np_base_release, .show_attribute = target_fabric_np_base_attr_show, .store_attribute = target_fabric_np_base_attr_store, }; @@ -466,6 +491,7 @@ static struct config_group *target_fabric_make_np( if (!(se_tpg_np) || IS_ERR(se_tpg_np)) return ERR_PTR(-EINVAL); + se_tpg_np->tpg_np_parent = se_tpg; config_group_init_type_name(&se_tpg_np->tpg_np_group, name, &TF_CIT_TMPL(tf)->tfc_tpg_np_base_cit); @@ -476,14 +502,10 @@ static void target_fabric_drop_np( struct config_group *group, struct config_item *item) { - struct se_portal_group *se_tpg = container_of(group, - struct se_portal_group, tpg_np_group); - struct target_fabric_configfs *tf = se_tpg->se_tpg_wwn->wwn_tf; - struct se_tpg_np *se_tpg_np = container_of(to_config_group(item), - struct se_tpg_np, tpg_np_group); - + /* + * struct se_tpg_np is released via target_fabric_np_base_release() + */ config_item_put(item); - tf->tf_ops.fabric_drop_np(se_tpg_np); } static struct configfs_group_operations target_fabric_np_group_ops = { @@ -814,7 +836,18 @@ TF_CIT_SETUP(tpg_param, &target_fabric_tpg_param_item_ops, NULL, NULL); */ CONFIGFS_EATTR_OPS(target_fabric_tpg, se_portal_group, tpg_group); +static void target_fabric_tpg_release(struct config_item *item) +{ + struct se_portal_group *se_tpg = container_of(to_config_group(item), + struct se_portal_group, tpg_group); + struct se_wwn *wwn = se_tpg->se_tpg_wwn; + struct target_fabric_configfs *tf = wwn->wwn_tf; + + tf->tf_ops.fabric_drop_tpg(se_tpg); +} + static struct configfs_item_operations target_fabric_tpg_base_item_ops = { + .release = target_fabric_tpg_release, .show_attribute = target_fabric_tpg_attr_show, .store_attribute = target_fabric_tpg_attr_store, }; @@ -872,8 +905,6 @@ static void target_fabric_drop_tpg( struct config_group *group, struct config_item *item) { - struct se_wwn *wwn = container_of(group, struct se_wwn, wwn_group); - struct target_fabric_configfs *tf = wwn->wwn_tf; struct se_portal_group *se_tpg = container_of(to_config_group(item), struct se_portal_group, tpg_group); struct config_group *tpg_cg = &se_tpg->tpg_group; @@ -890,15 +921,28 @@ static void target_fabric_drop_tpg( } config_item_put(item); - tf->tf_ops.fabric_drop_tpg(se_tpg); } +static void target_fabric_release_wwn(struct config_item *item) +{ + struct se_wwn *wwn = container_of(to_config_group(item), + struct se_wwn, wwn_group); + struct target_fabric_configfs *tf = wwn->wwn_tf; + + tf->tf_ops.fabric_drop_wwn(wwn); +} + +static struct configfs_item_operations target_fabric_tpg_item_ops = { + .release = target_fabric_release_wwn, +}; + static struct configfs_group_operations target_fabric_tpg_group_ops = { .make_group = target_fabric_make_tpg, .drop_item = target_fabric_drop_tpg, }; -TF_CIT_SETUP(tpg, NULL, &target_fabric_tpg_group_ops, NULL); +TF_CIT_SETUP(tpg, &target_fabric_tpg_item_ops, &target_fabric_tpg_group_ops, + NULL); /* End of tfc_tpg_cit */ @@ -932,13 +976,7 @@ static void target_fabric_drop_wwn( struct config_group *group, struct config_item *item) { - struct target_fabric_configfs *tf = container_of(group, - struct target_fabric_configfs, tf_group); - struct se_wwn *wwn = container_of(to_config_group(item), - struct se_wwn, wwn_group); - config_item_put(item); - tf->tf_ops.fabric_drop_wwn(wwn); } static struct configfs_group_operations target_fabric_wwn_group_ops = { diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 9f926556918..0828b6c8610 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -882,6 +882,7 @@ struct se_port { } ____cacheline_aligned; struct se_tpg_np { + struct se_portal_group *tpg_np_parent; struct config_group tpg_np_group; } ____cacheline_aligned; -- 2.20.1