* [PATCH v7 1/4] fs: configfs: add helpers for opening non-configfs paths
2026-09-27 8:20 [PATCH v7 0/4] nvmet: avoid recursive configfs open Runyu Xiao
@ 2026-09-27 8:20 ` Runyu Xiao
2026-09-27 8:39 ` sashiko-bot
2026-09-27 8:20 ` [PATCH v7 2/4] nvmet: avoid recursive configfs open for file-backed namespaces Runyu Xiao
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Runyu Xiao @ 2026-09-27 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Breno Leitao, Andreas Hindborg, Sagi Grimberg, Chaitanya Kulkarni,
Keith Busch, Logan Gunthorpe, Martin K . Petersen, Lee Duncan,
Hannes Reinecke, Nicholas Bellinger, linux-nvme, linux-scsi,
target-devel, linux-kernel, stable, Runyu Xiao, Jianhao Xu
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 <runyu.xiao@seu.edu.cn>
---
fs/configfs/mount.c | 57 ++++++++++++++++++++++++++++++++++++++++
include/linux/configfs.h | 5 ++++
2 files changed, 62 insertions(+)
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index d8cac1cbf3bd5..aa79be05dd5bc 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/mount.h>
#include <linux/fs_context.h>
+#include <linux/namei.h>
#include <linux/pagemap.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -118,6 +119,62 @@ 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. O_NOFOLLOW is honored
+ * when resolving the final path component.
+ *
+ * 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;
+ unsigned int lookup_flags = 0;
+ int ret;
+
+ if (!(flags & O_NOFOLLOW))
+ lookup_flags = LOOKUP_FOLLOW;
+ ret = kern_path(filename, lookup_flags, &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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 4/4] scsi: target: pin db_root for metadata writes
2026-09-27 8:20 [PATCH v7 0/4] nvmet: avoid recursive configfs open Runyu Xiao
` (2 preceding siblings ...)
2026-09-27 8:20 ` [PATCH v7 3/4] nvmet: avoid recursive configfs open for passthru Runyu Xiao
@ 2026-09-27 8:20 ` Runyu Xiao
2026-09-28 5:12 ` [PATCH v7 0/4] nvmet: avoid recursive configfs open Christoph Hellwig
4 siblings, 0 replies; 7+ messages in thread
From: Runyu Xiao @ 2026-09-27 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Breno Leitao, Andreas Hindborg, Sagi Grimberg, Chaitanya Kulkarni,
Keith Busch, Logan Gunthorpe, Martin K . Petersen, Lee Duncan,
Hannes Reinecke, Nicholas Bellinger, linux-nvme, linux-scsi,
target-devel, linux-kernel, stable, Runyu Xiao, Jianhao Xu
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.
Protect db_root and db_root_path with a dedicated mutex while metadata
paths are opened or the configured root is replaced. Return an error if
target devices appear before a validated root can be published.
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 <runyu.xiao@seu.edu.cn>
---
drivers/target/target_core_alua.c | 42 ++++----
drivers/target/target_core_configfs.c | 142 +++++++++++++++++++++-----
drivers/target/target_core_internal.h | 5 +
drivers/target/target_core_pr.c | 19 ++--
4 files changed, 153 insertions(+), 55 deletions(-)
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 140154d93c430..c39e57c1ebf9e 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -18,7 +18,6 @@
#include <linux/fcntl.h>
#include <linux/file.h>
#include <linux/fs.h>
-#include <linux/fs_struct.h>
#include <linux/kthread.h>
#include <scsi/scsi_proto.h>
#include <linux/unaligned.h>
@@ -862,15 +861,18 @@ 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);
+ mutex_lock(&db_root_lock);
+ if (!db_root_path.dentry) {
+ mutex_unlock(&db_root_lock);
+ pr_err("db_root is not initialized for ALUA metadata path: %s\n",
+ path);
+ return -ENODEV;
}
-
+ file = configfs_open_root(&db_root_path, path,
+ O_RDWR | O_CREAT | O_TRUNC, 0600);
+ mutex_unlock(&db_root_lock);
if (IS_ERR(file)) {
- pr_err("filp_open(%s) for ALUA metadata failed\n", path);
+ pr_err("configfs_open_root(%s) for ALUA metadata failed\n", path);
return -ENODEV;
}
ret = kernel_write(file, md_buf, md_buf_len, &pos);
@@ -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..31cd1ed42b135 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -96,57 +96,126 @@ 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;
+/* Protect db_root and db_root_path. */
+DEFINE_MUTEX(db_root_lock);
+
+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)
{
- return sprintf(page, "%s\n", db_root);
+ ssize_t ret;
+
+ mutex_lock(&db_root_lock);
+ ret = sprintf(page, "%s\n", db_root);
+ mutex_unlock(&db_root_lock);
+
+ return ret;
}
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");
+ r = -EINVAL;
+ goto unlock_put;
}
- path_put(&path);
+ mutex_lock(&db_root_lock);
+ 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);
+ mutex_unlock(&db_root_lock);
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,23 +3791,37 @@ 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);
+ mutex_lock(&db_root_lock);
+ db_root_path = path;
strscpy(db_root, db_root_stage);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
+ mutex_unlock(&db_root_lock);
+}
+
+static void target_release_db_root(void)
+{
+ struct path path;
+
+ mutex_lock(&db_root_lock);
+ path = db_root_path;
+ db_root_path = (struct path){};
+ mutex_unlock(&db_root_lock);
+
+ if (path.dentry)
+ path_put(&path);
}
static int __init target_core_init_configfs(void)
@@ -3797,6 +3880,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 +3908,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 +3916,7 @@ static int __init target_core_init_configfs(void)
core_dev_release_virtual_lun0();
rd_module_exit();
out_global:
+ target_release_db_root();
if (default_lu_gp) {
core_alua_free_lu_gp(default_lu_gp);
default_lu_gp = NULL;
@@ -3861,6 +3946,7 @@ static void __exit target_core_exit_configfs(void)
core_dev_release_virtual_lun0();
rd_module_exit();
target_xcopy_release_pt();
+ target_release_db_root();
release_se_kmem_caches();
}
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index f0886ea290345..502788ba732dd 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -4,6 +4,7 @@
#include <linux/configfs.h>
#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/types.h>
#include <target/target_core_base.h>
@@ -171,6 +172,10 @@ 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;
+extern struct mutex db_root_lock;
#endif /* TARGET_CORE_INTERNAL_H */
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 25b1bcacc0c8f..36d20732382f1 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -18,7 +18,6 @@
#include <linux/file.h>
#include <linux/fcntl.h>
#include <linux/fs.h>
-#include <linux/fs_struct.h>
#include <scsi/scsi_proto.h>
#include <linux/unaligned.h>
@@ -1965,16 +1964,22 @@ 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);
+ mutex_lock(&db_root_lock);
+ if (!db_root_path.dentry) {
+ mutex_unlock(&db_root_lock);
+ pr_err("db_root is not initialized for APTPL metadata path: %s\n",
+ path);
+ kfree(path);
+ return -ENODEV;
+ }
+ file = configfs_open_root(&db_root_path, path, flags, 0600);
+ mutex_unlock(&db_root_lock);
if (IS_ERR(file)) {
- pr_err("filp_open(%s) for APTPL metadata"
- " failed\n", path);
+ pr_err("configfs_open_root(%s) for APTPL metadata failed\n", path);
kfree(path);
return PTR_ERR(file);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread