Configfs store callbacks hold frag_sem while they run. Reopening a path that resolves to configfs from such a callback can acquire the same non-recursive semaphore again. Add configfs_file_open() for configured pathnames and configfs_open_root() for paths relative to a resolved root. Reject configfs roots before using file_open_root() so the normal open checks remain in place. Assisted-by: LLM Codex Signed-off-by: Runyu Xiao --- fs/configfs/mount.c | 50 ++++++++++++++++++++++++++++++++++++++++ include/linux/configfs.h | 5 ++++ 2 files changed, 58 insertions(+) diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c index d8cac1cbf3bd5..8d3c809676640 100644 --- a/fs/configfs/mount.c +++ b/fs/configfs/mount.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -118,6 +119,58 @@ static struct file_system_type configfs_fs_type = { }; MODULE_ALIAS_FS("configfs"); +/** + * configfs_open_root - open a path below a non-configfs root + * @root: resolved root path + * @name: path relative to @root + * @flags: open flags + * @mode: mode for a newly created file + * + * Use this from configfs store callbacks with a resolved non-configfs root + * to avoid re-entering configfs while the callback holds its fragment + * semaphore. + * + * Return: opened file, or an ERR_PTR() value. Returns -EINVAL if @root + * is on configfs. + */ +struct file *configfs_open_root(const struct path *root, const char *name, + int flags, umode_t mode) +{ + if (root->dentry->d_sb->s_type == &configfs_fs_type) + return ERR_PTR(-EINVAL); + + return file_open_root(root, name, flags, mode); +} +EXPORT_SYMBOL_GPL(configfs_open_root); + +/** + * configfs_file_open - open an existing non-configfs pathname + * @filename: pathname to open; it must already exist + * @flags: open flags for the existing pathname + * @mode: unused; creation is not supported + * + * Resolve @filename and reject configfs paths. Use this from configfs + * store callbacks for existing configured paths. + * + * Return: opened file, or an ERR_PTR() value. Returns -EINVAL if the + * resolved path is on configfs. + */ +struct file *configfs_file_open(const char *filename, int flags, umode_t mode) +{ + struct file *file; + struct path path; + int ret; + + ret = kern_path(filename, LOOKUP_FOLLOW, &path); + if (ret) + return ERR_PTR(ret); + + file = configfs_open_root(&path, "", flags, mode); + path_put(&path); + return file; +} +EXPORT_SYMBOL_GPL(configfs_file_open); + struct dentry *configfs_pin_fs(void) { int err = simple_pin_fs(&configfs_fs_type, &configfs_mount, diff --git a/include/linux/configfs.h b/include/linux/configfs.h index ef65c75beeaad..2a803bb836b4d 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -34,6 +34,8 @@ struct configfs_group_operations; struct configfs_attribute; struct configfs_bin_attribute; struct configfs_subsystem; +struct file; +struct path; struct config_item { char *ci_name; @@ -243,6 +245,9 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys); int configfs_register_group(struct config_group *parent_group, struct config_group *group); void configfs_unregister_group(struct config_group *group); +struct file *configfs_open_root(const struct path *root, const char *name, + int flags, umode_t mode); +struct file *configfs_file_open(const char *filename, int flags, umode_t mode); void configfs_remove_default_groups(struct config_group *group); -- 2.34.1 nvmet_ns_enable_store() runs as a configfs store callback while configfs holds the item frag_sem. File-backed namespace enable used filp_open() on the configured device_path, so a path into configfs could re-enter __configfs_open_file() and try to acquire the same semaphore again. Use configfs_file_open() so the path is resolved before opening, configfs-backed paths are rejected, and the resolved path is opened with file_open_root() while retaining the normal open-time permission and security checks. Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support") Cc: stable@vger.kernel.org Reviewed-by: Christoph Hellwig Assisted-by: LLM Codex Signed-off-by: Runyu Xiao --- drivers/nvme/target/io-cmd-file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c index 0b22d183f9279..2a4f25de94ba1 100644 --- a/drivers/nvme/target/io-cmd-file.c +++ b/drivers/nvme/target/io-cmd-file.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "nvmet.h" @@ -38,7 +39,7 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns) if (!ns->buffered_io) flags |= O_DIRECT; - ns->file = filp_open(ns->device_path, flags, 0); + ns->file = configfs_file_open(ns->device_path, flags, 0); if (IS_ERR(ns->file)) { ret = PTR_ERR(ns->file); pr_err("failed to open file %s: (%d)\n", -- 2.34.1 nvmet_passthru_enable_store() runs as a configfs store callback while configfs holds the item frag_sem. Passthru enable used filp_open() on the configured controller path, so a path into configfs could re-enter __configfs_open_file() and try to acquire the same semaphore again. Use configfs_file_open() so the path is resolved before opening, configfs-backed paths are rejected, and the resolved path is opened with file_open_root() while retaining the normal open-time permission and security checks. Fixes: cae5b01a2afc ("nvmet: introduce the passthru configfs interface") Cc: stable@vger.kernel.org Reviewed-by: Christoph Hellwig Assisted-by: LLM Codex Signed-off-by: Runyu Xiao --- drivers/nvme/target/passthru.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c index fa6527c537e26..d60256004e6cf 100644 --- a/drivers/nvme/target/passthru.c +++ b/drivers/nvme/target/passthru.c @@ -9,6 +9,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include #include "../host/nvme.h" #include "nvmet.h" @@ -602,7 +603,7 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) goto out_unlock; } - file = filp_open(subsys->passthru_ctrl_path, O_RDWR, 0); + file = configfs_file_open(subsys->passthru_ctrl_path, O_RDWR, 0); if (IS_ERR(file)) { ret = PTR_ERR(file); goto out_unlock; -- 2.34.1 ALUA and persistent reservation metadata files are derived from the configurable db_root string and opened with filp_open(). If db_root points at configfs, a metadata update from a configfs store callback can re-enter configfs while the callback still holds frag_sem. A one-time pathname check can also be bypassed by retargeting a symlink. Resolve db_root once and retain the resulting path while target devices use it. Use configfs_open_root() both to reject configfs roots and to open metadata files relative to the pinned root. Resolve a new root outside target_devices_lock and recheck the device count before publishing it, so the path walk does not occur under that lock. Fixes: fdddf932269a ("target: use new "dbroot" target attribute") Link: https://lore.kernel.org/r/20260818051442.1523210-1-runyu.xiao@seu.edu.cn Cc: stable@vger.kernel.org Assisted-by: LLM Codex Signed-off-by: Runyu Xiao --- drivers/target/target_core_alua.c | 42 +++++----- drivers/target/target_core_configfs.c | 116 ++++++++++++++++++++------ drivers/target/target_core_internal.h | 3 + drivers/target/target_core_pr.c | 21 +++-- 4 files changed, 127 insertions(+), 55 deletions(-) diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 140154d93c430..601581f2071ff 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -862,20 +861,23 @@ static int core_alua_write_tpg_metadata( loff_t pos = 0; int ret; - if (tsk_is_kthread(current)) { - scoped_with_init_fs() - file = filp_open(path, O_RDWR | O_CREAT | O_TRUNC, 0600); - } else { - file = filp_open(path, O_RDWR | O_CREAT | O_TRUNC, 0600); + if (!db_root_path.dentry) { + pr_err("db_root is not initialized for ALUA metadata path: %s/%s\n", + db_root, path); + return -ENODEV; } + file = configfs_open_root(&db_root_path, path, + O_RDWR | O_CREAT | O_TRUNC, 0600); if (IS_ERR(file)) { - pr_err("filp_open(%s) for ALUA metadata failed\n", path); + pr_err("configfs_open_root(%s/%s) for ALUA metadata failed\n", + db_root, path); return -ENODEV; } ret = kernel_write(file, md_buf, md_buf_len, &pos); if (ret < 0) - pr_err("Error writing ALUA metadata file: %s\n", path); + pr_err("Error writing ALUA metadata file: %s/%s\n", db_root, + path); fput(file); return (ret < 0) ? -EIO : 0; } @@ -905,9 +907,9 @@ static int core_alua_update_tpg_primary_metadata( tg_pt_gp->tg_pt_gp_alua_access_status); rc = -ENOMEM; - path = kasprintf(GFP_KERNEL, "%s/alua/tpgs_%s/%s", db_root, - &wwn->unit_serial[0], - config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item)); + path = kasprintf(GFP_KERNEL, "alua/tpgs_%s/%s", + &wwn->unit_serial[0], + config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item)); if (path) { rc = core_alua_write_tpg_metadata(path, md_buf, len); kfree(path); @@ -1196,16 +1198,16 @@ static int core_alua_update_tpg_secondary_metadata(struct se_lun *lun) lun->lun_tg_pt_secondary_stat); if (se_tpg->se_tpg_tfo->tpg_get_tag != NULL) { - path = kasprintf(GFP_KERNEL, "%s/alua/%s/%s+%hu/lun_%llu", - db_root, se_tpg->se_tpg_tfo->fabric_name, - se_tpg->se_tpg_tfo->tpg_get_wwn(se_tpg), - se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg), - lun->unpacked_lun); + path = kasprintf(GFP_KERNEL, "alua/%s/%s+%hu/lun_%llu", + se_tpg->se_tpg_tfo->fabric_name, + se_tpg->se_tpg_tfo->tpg_get_wwn(se_tpg), + se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg), + lun->unpacked_lun); } else { - path = kasprintf(GFP_KERNEL, "%s/alua/%s/%s/lun_%llu", - db_root, se_tpg->se_tpg_tfo->fabric_name, - se_tpg->se_tpg_tfo->tpg_get_wwn(se_tpg), - lun->unpacked_lun); + path = kasprintf(GFP_KERNEL, "alua/%s/%s/lun_%llu", + se_tpg->se_tpg_tfo->fabric_name, + se_tpg->se_tpg_tfo->tpg_get_wwn(se_tpg), + lun->unpacked_lun); } if (!path) { rc = -ENOMEM; diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 2b19a956007b7..db1c56835f077 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -96,7 +96,41 @@ static ssize_t target_core_item_version_show(struct config_item *item, CONFIGFS_ATTR_RO(target_core_item_, version); char db_root[DB_ROOT_LEN] = DB_ROOT_DEFAULT; -static char db_root_stage[DB_ROOT_LEN]; +struct path db_root_path; + +static int target_validate_db_root(const char *path_str, struct path *path) +{ + struct file *file; + int ret; + + ret = kern_path(path_str, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, path); + if (ret) { + pr_err("db_root: cannot open: %s\n", path_str); + if (ret == -ENOTDIR) + pr_err("db_root: not a directory: %s\n", path_str); + return ret; + } + + file = configfs_open_root(path, "", O_RDONLY, 0); + if (IS_ERR(file)) { + ret = PTR_ERR(file); + path_put(path); + *path = (struct path){}; + if (ret != -EINVAL) + return ret; + + pr_err("db_root: configfs is not a valid target database root: %s\n", + path_str); + return -EINVAL; + } + + path_put(path); + *path = file->f_path; + path_get(path); + fput(file); + + return 0; +} static ssize_t target_core_item_dbroot_show(struct config_item *item, char *page) @@ -107,46 +141,70 @@ static ssize_t target_core_item_dbroot_show(struct config_item *item, static ssize_t target_core_item_dbroot_store(struct config_item *item, const char *page, size_t count) { + char *db_root_stage; ssize_t read_bytes; ssize_t r = -EINVAL; struct path path = {}; + struct path old_path = {}; + bool have_old_path = false; mutex_lock(&target_devices_lock); if (target_devices) { pr_err("db_root: cannot be changed because it's in use\n"); - goto unlock; + mutex_unlock(&target_devices_lock); + return r; } + mutex_unlock(&target_devices_lock); if (count > (DB_ROOT_LEN - 1)) { pr_err("db_root: count %d exceeds DB_ROOT_LEN-1: %u\n", (int)count, DB_ROOT_LEN - 1); - goto unlock; + return r; } + db_root_stage = kmalloc(DB_ROOT_LEN, GFP_KERNEL); + if (!db_root_stage) + return -ENOMEM; + read_bytes = scnprintf(db_root_stage, DB_ROOT_LEN, "%s", page); if (!read_bytes) - goto unlock; + goto free_stage; if (db_root_stage[read_bytes - 1] == '\n') db_root_stage[read_bytes - 1] = '\0'; /* validate new db root before accepting it */ - r = kern_path(db_root_stage, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &path); - if (r) { - pr_err("db_root: cannot open: %s\n", db_root_stage); - if (r == -ENOTDIR) - pr_err("db_root: not a directory: %s\n", db_root_stage); - goto unlock; + r = target_validate_db_root(db_root_stage, &path); + if (r) + goto free_stage; + + mutex_lock(&target_devices_lock); + if (target_devices) { + pr_err("db_root: cannot be changed because it's in use\n"); + goto unlock_put; } - path_put(&path); + have_old_path = db_root_path.dentry; + if (have_old_path) + old_path = db_root_path; + db_root_path = path; + path = (struct path){}; strscpy(db_root, db_root_stage); pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root); r = read_bytes; -unlock: +unlock_put: mutex_unlock(&target_devices_lock); + if (path.dentry) + path_put(&path); + if (have_old_path) + path_put(&old_path); + kfree(db_root_stage); + return r; + +free_stage: + kfree(db_root_stage); return r; } @@ -3722,21 +3780,20 @@ void target_setup_backend_cits(struct target_backend *tb) static void target_init_dbroot(void) { - struct file *fp; + const char *db_root_stage; + struct path path = {}; + int ret; - snprintf(db_root_stage, DB_ROOT_LEN, DB_ROOT_PREFERRED); - fp = filp_open(db_root_stage, O_RDONLY, 0); - if (IS_ERR(fp)) { - pr_err("db_root: cannot open: %s\n", db_root_stage); - return; - } - if (!S_ISDIR(file_inode(fp)->i_mode)) { - filp_close(fp, NULL); - pr_err("db_root: not a valid directory: %s\n", db_root_stage); - return; + db_root_stage = DB_ROOT_PREFERRED; + ret = target_validate_db_root(db_root_stage, &path); + if (ret) { + db_root_stage = DB_ROOT_DEFAULT; + ret = target_validate_db_root(db_root_stage, &path); + if (ret) + return; } - filp_close(fp, NULL); + db_root_path = path; strscpy(db_root, db_root_stage); pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root); } @@ -3797,6 +3854,10 @@ static int __init target_core_init_configfs(void) /* * Register the target_core_mod subsystem with configfs. */ + /* Resolve db_root before making the configfs attributes visible. */ + scoped_with_kernel_creds() + target_init_dbroot(); + ret = configfs_register_subsystem(subsys); if (ret < 0) { pr_err("Error %d while registering subsystem %s\n", @@ -3821,9 +3882,6 @@ static int __init target_core_init_configfs(void) if (ret < 0) goto out; - scoped_with_kernel_creds() - target_init_dbroot(); - return 0; out: @@ -3832,6 +3890,8 @@ static int __init target_core_init_configfs(void) core_dev_release_virtual_lun0(); rd_module_exit(); out_global: + if (db_root_path.dentry) + path_put(&db_root_path); if (default_lu_gp) { core_alua_free_lu_gp(default_lu_gp); default_lu_gp = NULL; @@ -3861,6 +3921,8 @@ static void __exit target_core_exit_configfs(void) core_dev_release_virtual_lun0(); rd_module_exit(); target_xcopy_release_pt(); + if (db_root_path.dentry) + path_put(&db_root_path); release_se_kmem_caches(); } diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index f0886ea290345..c3e55f60cfb11 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -171,6 +171,9 @@ extern struct se_portal_group xcopy_pt_tpg; #define DB_ROOT_DEFAULT "/var/target" #define DB_ROOT_PREFERRED "/etc/target" +struct path; + extern char db_root[]; +extern struct path db_root_path; #endif /* TARGET_CORE_INTERNAL_H */ diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 25b1bcacc0c8f..0628622d916ba 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include @@ -1965,16 +1964,21 @@ static int __core_scsi3_write_aptpl_to_file( int ret; loff_t pos = 0; - path = kasprintf(GFP_KERNEL, "%s/pr/aptpl_%s", db_root, - &wwn->unit_serial[0]); + path = kasprintf(GFP_KERNEL, "pr/aptpl_%s", &wwn->unit_serial[0]); if (!path) return -ENOMEM; - scoped_with_init_fs() - file = filp_open(path, flags, 0600); + if (!db_root_path.dentry) { + pr_err("db_root is not initialized for APTPL metadata path: %s/%s\n", + db_root, path); + kfree(path); + return -ENODEV; + } + + file = configfs_open_root(&db_root_path, path, flags, 0600); if (IS_ERR(file)) { - pr_err("filp_open(%s) for APTPL metadata" - " failed\n", path); + pr_err("configfs_open_root(%s/%s) for APTPL metadata failed\n", + db_root, path); kfree(path); return PTR_ERR(file); } @@ -1984,7 +1988,8 @@ static int __core_scsi3_write_aptpl_to_file( ret = kernel_write(file, buf, pr_aptpl_buf_len, &pos); if (ret < 0) - pr_debug("Error writing APTPL metadata file: %s\n", path); + pr_debug("Error writing APTPL metadata file: %s/%s\n", db_root, + path); fput(file); kfree(path); -- 2.34.1