On 3/28/22 5:39 AM, Maurizio Lombardi wrote:
> The target driver prevents the users from changing
> the database root directory if a target module like ib_srpt has
> been registered, this makes it difficult for users to
> set their preferred database directory if the module
> gets loaded during the system boot.
>
> Let the users modify dbroot if there are no registered devices
>
> v2: add a mutex to protect against possible race conditions
>
> Signed-off-by: Maurizio Lombardi <mlombard@xxxxxxxxxx>
> ---
> drivers/target/target_core_configfs.c | 47 ++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index 023bd4516a68..fed1e7bdc013 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -72,6 +72,9 @@ static struct config_group target_core_hbagroup;
> static struct config_group alua_group;
> static struct config_group alua_lu_gps_group;
>
> +static unsigned int target_devices;
> +static DEFINE_MUTEX(target_devices_lock);
> +
> static inline struct se_hba *
> item_to_hba(struct config_item *item)
> {
> @@ -105,51 +108,48 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> {
> ssize_t read_bytes;
> struct file *fp;
> + ssize_t r = -EINVAL;
>
> - mutex_lock(&g_tf_lock);
> - if (!list_empty(&g_tf_list)) {
> - mutex_unlock(&g_tf_lock);
> - pr_err("db_root: cannot be changed: target drivers registered");
> - return -EINVAL;
> + mutex_lock(&target_devices_lock);
> + if (target_devices) {
> + pr_err("db_root: cannot be changed because it's in use\n");
> + goto unlock;
> }
>
> if (count > (DB_ROOT_LEN - 1)) {
> - mutex_unlock(&g_tf_lock);
> pr_err("db_root: count %d exceeds DB_ROOT_LEN-1: %u\n",
> (int)count, DB_ROOT_LEN - 1);
> - return -EINVAL;
> + goto unlock;
> }
>
> read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
> - if (!read_bytes) {
> - mutex_unlock(&g_tf_lock);
> - return -EINVAL;
> - }
> + if (!read_bytes)
> + goto unlock;
> +
> if (db_root_stage[read_bytes - 1] == '\n')
> db_root_stage[read_bytes - 1] = '\0';
>
> /* validate new db root before accepting it */
> fp = filp_open(db_root_stage, O_RDONLY, 0);
> if (IS_ERR(fp)) {
> - mutex_unlock(&g_tf_lock);
> pr_err("db_root: cannot open: %s\n", db_root_stage);
> - return -EINVAL;
> + goto unlock;
> }
> if (!S_ISDIR(file_inode(fp)->i_mode)) {
> filp_close(fp, NULL);
> - mutex_unlock(&g_tf_lock);
> pr_err("db_root: not a directory: %s\n", db_root_stage);
> - return -EINVAL;
> + goto unlock;
> }
> filp_close(fp, NULL);
>
> strncpy(db_root, db_root_stage, read_bytes);
> -
> - mutex_unlock(&g_tf_lock);
> -
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>
> - return read_bytes;
> + r = read_bytes;
> +
> +unlock:
> + mutex_unlock(&target_devices_lock);
> + return r;
> }
>
> CONFIGFS_ATTR(target_core_item_, dbroot);
> @@ -3315,6 +3315,10 @@ static struct config_group *target_core_make_subdev(
> */
> target_stat_setup_dev_default_groups(dev);
>
> + mutex_lock(&target_devices_lock);
> + target_devices++;
> + mutex_unlock(&target_devices_lock);
> +
> mutex_unlock(&hba->hba_access_mutex);
> return &dev->dev_group;
>
> @@ -3353,6 +3357,11 @@ static void target_core_drop_subdev(
> * se_dev is released from target_core_dev_item_ops->release()
> */
> config_item_put(item);
> +
> + mutex_lock(&target_devices_lock);
> + target_devices--;
> + mutex_unlock(&target_devices_lock);
> +
> mutex_unlock(&hba->hba_access_mutex);
> }
>
Reviewed-by: Mike Christie <michael.christie@xxxxxxxxxx>
