[PATCH 0/2] nvmet: avoid configfs recursion when enabling backends

[PATCH 0/2] nvmet: avoid configfs recursion when enabling backends

[Runyu Xiao]

The nvmet configfs store callbacks run while configfs holds the item's
frag_sem. Two enable paths then call filp_open() on user-controlled
paths:

- file-backed namespace enable via device_path
- passthru controller enable via passthru_ctrl_path

If either path points back into configfs, the open path re-enters
__configfs_open_file() and tries to take the same frag_sem again.

Resolve the configured path before opening it, reject configfs-backed
paths, and use dentry_open() on the resolved path. This prevents the
configfs recursion without changing valid backend users.

Runyu Xiao (2):
  nvmet: avoid recursive configfs open for file-backed namespaces
  nvmet: avoid recursive configfs open for passthru

 drivers/nvme/target/io-cmd-file.c | 19 ++++++++++++++++++-
 drivers/nvme/target/passthru.c    | 17 ++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/2] nvmet: avoid recursive configfs open for file-backed namespaces

[Runyu Xiao]

nvmet_ns_enable_store() runs as a configfs store callback while configfs
holds the item's frag_sem. For file-backed namespaces, nvmet_ns_enable()
calls nvmet_file_ns_enable(), which uses filp_open() on the
user-supplied device_path.

If device_path points back into configfs, the open path re-enters
__configfs_open_file() and tries to take the same frag_sem again.

Resolve the path with kern_path(), reject configfs paths, and open the
resolved path with dentry_open() instead of filp_open(). This keeps
valid block-device and regular-file backends working without
re-entering configfs.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Cc: stable@vger.kernel.org

Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/nvme/target/io-cmd-file.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 2d068439b129..6d653519327a 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -9,6 +9,7 @@
 #include <linux/falloc.h>
 #include <linux/file.h>
 #include <linux/fs.h>
+#include <linux/namei.h>
 #include "nvmet.h"
 
 #define NVMET_MIN_MPOOL_OBJ		16
@@ -33,12 +34,28 @@ void nvmet_file_ns_disable(struct nvmet_ns *ns)
 int nvmet_file_ns_enable(struct nvmet_ns *ns)
 {
 	int flags = O_RDWR | O_LARGEFILE;
+	struct path path;
 	int ret = 0;
 
 	if (!ns->buffered_io)
 		flags |= O_DIRECT;
 
-	ns->file = filp_open(ns->device_path, flags, 0);
+	ret = kern_path(ns->device_path, LOOKUP_FOLLOW, &path);
+	if (ret) {
+		pr_err("failed to open file %s: (%d)\n",
+		       ns->device_path, ret);
+		return ret;
+	}
+
+	if (!strcmp(path.dentry->d_sb->s_type->name, "configfs")) {
+		pr_err("configfs paths cannot back namespace %s\n",
+		       ns->device_path);
+		path_put(&path);
+		return -EINVAL;
+	}
+
+	ns->file = dentry_open(&path, flags, current_cred());
+	path_put(&path);
 	if (IS_ERR(ns->file)) {
 		ret = PTR_ERR(ns->file);
 		pr_err("failed to open file %s: (%d)\n",
-- 
2.34.1

[PATCH 2/2] nvmet: avoid recursive configfs open for passthru

[Runyu Xiao]

nvmet_passthru_enable_store() runs as a configfs store callback while
configfs holds the item's frag_sem. nvmet_passthru_ctrl_enable() then
uses filp_open() on the configured passthru_ctrl_path.

If passthru_ctrl_path points back into configfs, the open path
re-enters __configfs_open_file() and tries to take the same frag_sem
again.

Resolve the path with kern_path(), reject configfs paths, and open the
resolved path with dentry_open() instead of filp_open(). Configfs paths
are not valid passthru controller backends, so rejecting them avoids
the recursion without changing valid users.

Fixes: cae5b01a2afc ("nvmet: introduce the passthru configfs interface")
Cc: stable@vger.kernel.org

Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/nvme/target/passthru.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 0c361b1e3566..be302f70d0e2 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -8,6 +8,7 @@
  *
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/namei.h>
 #include <linux/module.h>
 
 #include "../host/nvme.h"
@@ -578,6 +579,7 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 {
 	struct nvme_ctrl *ctrl;
 	struct file *file;
+	struct path path;
 	int ret = -EINVAL;
 	void *old;
 
@@ -592,7 +594,20 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 		goto out_unlock;
 	}
 
-	file = filp_open(subsys->passthru_ctrl_path, O_RDWR, 0);
+	ret = kern_path(subsys->passthru_ctrl_path, LOOKUP_FOLLOW, &path);
+	if (ret)
+		goto out_unlock;
+
+	if (!strcmp(path.dentry->d_sb->s_type->name, "configfs")) {
+		pr_err("configfs paths cannot back passthru controller %s\n",
+		       subsys->passthru_ctrl_path);
+		path_put(&path);
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	file = dentry_open(&path, O_RDWR, current_cred());
+	path_put(&path);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto out_unlock;
-- 
2.34.1

Re: [PATCH 1/2] nvmet: avoid recursive configfs open for file-backed namespaces

[Christoph Hellwig]

> +	ret = kern_path(ns->device_path, LOOKUP_FOLLOW, &path);
> +	if (ret) {
> +		pr_err("failed to open file %s: (%d)\n",
> +		       ns->device_path, ret);
> +		return ret;
> +	}
> +
> +	if (!strcmp(path.dentry->d_sb->s_type->name, "configfs")) {
> +		pr_err("configfs paths cannot back namespace %s\n",
> +		       ns->device_path);
> +		path_put(&path);
> +		return -EINVAL;
> +	}

String comparisons are a bit weird, checking the actual file_system_type
is a lot cheaper and more safe.  And please move this into a helper
in configfs as I bet there are tons of other users like this and they'd
benefit fro ma common helper.

[PATCH v2 0/3] nvmet: avoid recursive configfs open

[Runyu Xiao]

This v2 moves the configfs path check into a shared helper so nvmet can
reject configfs-backed paths without string comparisons, then reuses it
from both nvmet open paths.

Runyu Xiao (3):
  fs: configfs: add helper to identify configfs paths
  nvmet: avoid recursive configfs open for file-backed namespaces
  nvmet: avoid recursive configfs open for passthru

 drivers/nvme/target/io-cmd-file.c | 20 +++++++++++++++++++-
 drivers/nvme/target/passthru.c    | 17 ++++++++++++++++-
 fs/configfs/mount.c               |  6 ++++++
 include/linux/configfs.h          |  2 ++
 4 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] fs: configfs: add helper to identify configfs paths

[Runyu Xiao]

Several nvmet paths need to identify configfs-backed resolved paths while
already running under configfs callbacks. Keep the classification in
configfs itself so callers can reuse the filesystem-type test instead of
open-coding string comparisons.

Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 fs/configfs/mount.c      | 6 ++++++
 include/linux/configfs.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index 456c4a2efb53..1204421b61df 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -120,6 +120,12 @@ static struct file_system_type configfs_fs_type = {
 };
 MODULE_ALIAS_FS("configfs");
 
+bool configfs_path_is_configfs(const struct path *path)
+{
+	return path->dentry->d_sb->s_type == &configfs_fs_type;
+}
+EXPORT_SYMBOL_GPL(configfs_path_is_configfs);
+
 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 698520b1bfdb..b3426c952835 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -34,6 +34,7 @@ struct configfs_group_operations;
 struct configfs_attribute;
 struct configfs_bin_attribute;
 struct configfs_subsystem;
+struct path;
 
 struct config_item {
 	char			*ci_name;
@@ -243,6 +244,7 @@ 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);
+bool configfs_path_is_configfs(const struct path *path);
 
 void configfs_remove_default_groups(struct config_group *group);
 
-- 
2.34.1

[PATCH v2 2/3] nvmet: avoid recursive configfs open for file-backed namespaces

[Runyu Xiao]

nvmet_ns_enable_store() runs under configfs frag_sem. If a file-backed
namespace path resolves into configfs, filp_open() can re-enter configfs
and recurse on the same semaphore.

Reject configfs-backed paths after kern_path() and open the resolved path
with dentry_open() instead.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/nvme/target/io-cmd-file.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 2d068439b129..4ba3ebb82e6a 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -8,7 +8,9 @@
 #include <linux/uio.h>
 #include <linux/falloc.h>
 #include <linux/file.h>
+#include <linux/configfs.h>
 #include <linux/fs.h>
+#include <linux/namei.h>
 #include "nvmet.h"
 
 #define NVMET_MIN_MPOOL_OBJ		16
@@ -33,12 +35,28 @@ void nvmet_file_ns_disable(struct nvmet_ns *ns)
 int nvmet_file_ns_enable(struct nvmet_ns *ns)
 {
 	int flags = O_RDWR | O_LARGEFILE;
+	struct path path;
 	int ret = 0;
 
 	if (!ns->buffered_io)
 		flags |= O_DIRECT;
 
-	ns->file = filp_open(ns->device_path, flags, 0);
+	ret = kern_path(ns->device_path, LOOKUP_FOLLOW, &path);
+	if (ret) {
+		pr_err("failed to open file %s: (%d)\n",
+		       ns->device_path, ret);
+		return ret;
+	}
+
+	if (configfs_path_is_configfs(&path)) {
+		pr_err("configfs paths cannot back namespace %s\n",
+		       ns->device_path);
+		path_put(&path);
+		return -EINVAL;
+	}
+
+	ns->file = dentry_open(&path, flags, current_cred());
+	path_put(&path);
 	if (IS_ERR(ns->file)) {
 		ret = PTR_ERR(ns->file);
 		pr_err("failed to open file %s: (%d)\n",
-- 
2.34.1

[PATCH v2 3/3] nvmet: avoid recursive configfs open for passthru

[Runyu Xiao]

nvmet_passthru_ctrl_enable() runs under configfs frag_sem. If the
configured passthru controller path resolves into configfs, filp_open()
can re-enter configfs and recurse on the same semaphore.

Reject configfs-backed paths after kern_path() and open the resolved path
with dentry_open() instead.

Fixes: cae5b01a2afc ("nvmet: introduce the passthru configfs interface")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/nvme/target/passthru.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 0c361b1e3566..b3c57bf033e1 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -9,6 +9,8 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
+#include <linux/configfs.h>
+#include <linux/namei.h>
 
 #include "../host/nvme.h"
 #include "nvmet.h"
@@ -578,6 +580,7 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 {
 	struct nvme_ctrl *ctrl;
 	struct file *file;
+	struct path path;
 	int ret = -EINVAL;
 	void *old;
 
@@ -592,7 +595,19 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 		goto out_unlock;
 	}
 
-	file = filp_open(subsys->passthru_ctrl_path, O_RDWR, 0);
+	ret = kern_path(subsys->passthru_ctrl_path, LOOKUP_FOLLOW, &path);
+	if (ret)
+		goto out_unlock;
+
+	if (configfs_path_is_configfs(&path)) {
+		pr_err("configfs paths cannot back passthru controller %s\n",
+		       subsys->passthru_ctrl_path);
+		path_put(&path);
+		goto out_unlock;
+	}
+
+	file = dentry_open(&path, O_RDWR, current_cred());
+	path_put(&path);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto out_unlock;
-- 
2.34.1

Re: [PATCH v2 1/3] fs: configfs: add helper to identify configfs paths

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH v2 2/3] nvmet: avoid recursive configfs open for file-backed namespaces

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH v2 3/3] nvmet: avoid recursive configfs open for passthru

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>