[PATCH 1/2] configfs: Add CONFIGFS_ATTR_PERM helper

[PATCH 1/2] configfs: Add CONFIGFS_ATTR_PERM helper

[Richard Weinberger]

This new helper allows creating rw files with custom
permissions.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/configfs.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index c771e9d0d0b9..698520b1bfdb 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -120,15 +120,19 @@ struct configfs_attribute {
 	ssize_t (*store)(struct config_item *, const char *, size_t);
 };
 
-#define CONFIGFS_ATTR(_pfx, _name)			\
+#define CONFIGFS_ATTR_PERM(_pfx, _name, _perm)		\
 static struct configfs_attribute _pfx##attr_##_name = {	\
 	.ca_name	= __stringify(_name),		\
-	.ca_mode	= S_IRUGO | S_IWUSR,		\
+	.ca_mode	= _perm,			\
 	.ca_owner	= THIS_MODULE,			\
 	.show		= _pfx##_name##_show,		\
 	.store		= _pfx##_name##_store,		\
 }
 
+#define CONFIGFS_ATTR(_pfx, _name) CONFIGFS_ATTR_PERM(	\
+		_pfx, _name, S_IRUGO | S_IWUSR		\
+)
+
 #define CONFIGFS_ATTR_RO(_pfx, _name)			\
 static struct configfs_attribute _pfx##attr_##_name = {	\
 	.ca_name	= __stringify(_name),		\
-- 
2.48.1

[PATCH 2/2] nvmet: Restrict in-band config files to root

[Richard Weinberger]

There is no need to have key material world readable.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/nvme/target/configfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e44ef69dffc2..7327543f161d 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -2128,7 +2128,7 @@ static ssize_t nvmet_host_dhchap_key_store(struct config_item *item,
 	return ret < 0 ? ret : count;
 }
 
-CONFIGFS_ATTR(nvmet_host_, dhchap_key);
+CONFIGFS_ATTR_PERM(nvmet_host_, dhchap_key, S_IRUSR | S_IWUSR);
 
 static ssize_t nvmet_host_dhchap_ctrl_key_show(struct config_item *item,
 		char *page)
@@ -2161,7 +2161,7 @@ static ssize_t nvmet_host_dhchap_ctrl_key_store(struct config_item *item,
 	return ret < 0 ? ret : count;
 }
 
-CONFIGFS_ATTR(nvmet_host_, dhchap_ctrl_key);
+CONFIGFS_ATTR_PERM(nvmet_host_, dhchap_ctrl_key, S_IRUSR | S_IWUSR);
 
 static ssize_t nvmet_host_dhchap_hash_show(struct config_item *item,
 		char *page)
@@ -2187,7 +2187,7 @@ static ssize_t nvmet_host_dhchap_hash_store(struct config_item *item,
 	return count;
 }
 
-CONFIGFS_ATTR(nvmet_host_, dhchap_hash);
+CONFIGFS_ATTR_PERM(nvmet_host_, dhchap_hash, S_IRUSR | S_IWUSR);
 
 static ssize_t nvmet_host_dhchap_dhgroup_show(struct config_item *item,
 		char *page)
@@ -2217,7 +2217,7 @@ static ssize_t nvmet_host_dhchap_dhgroup_store(struct config_item *item,
 	return count;
 }
 
-CONFIGFS_ATTR(nvmet_host_, dhchap_dhgroup);
+CONFIGFS_ATTR_PERM(nvmet_host_, dhchap_dhgroup, S_IRUSR | S_IWUSR);
 
 static struct configfs_attribute *nvmet_host_attrs[] = {
 	&nvmet_host_attr_dhchap_key,
-- 
2.48.1

Re: [PATCH 2/2] nvmet: Restrict in-band config files to root

[Chaitanya Kulkarni]

On 4/20/25 03:47, Richard Weinberger wrote:

There is no need to have key material world readable.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
  drivers/nvme/target/configfs.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e44ef69dffc2..7327543f161d 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -2128,7 +2128,7 @@ static ssize_t nvmet_host_dhchap_key_store(struct config_item *item,
  	return ret < 0 ? ret : count;
  }
  
-CONFIGFS_ATTR(nvmet_host_, dhchap_key);
+CONFIGFS_ATTR_PERM(nvmet_host_, dhchap_key, S_IRUSR | S_IWUSR);
  

Patch looks good to me, can we do something like this and
use in the rest of the patch ?

#define NVMET_CFGFS_ATTR_DEF_PERM (S_IRUSR | S_IWUSR)

or

Create a helper on the top of CONFIGFS_ATTR_PERM to wrap
permission into that for all the CONFIGFS_ATTR_PERM()

calls ?

-ck

Re: [PATCH 2/2] nvmet: Restrict in-band config files to root

[Richard Weinberger]

On Dienstag, 22. April 2025 08:32 'Chaitanya Kulkarni' via upstream wrote:
> Patch looks good to me, can we do something like this and
> use in the rest of the patch ?
> 
> #define NVMET_CFGFS_ATTR_DEF_PERM (S_IRUSR | S_IWUSR)
> 
> or
> 
> Create a helper on the top of CONFIGFS_ATTR_PERM to wrap
> permission into that for all the CONFIGFS_ATTR_PERM()

Sounds like you want to restrict read access to all nvmet configfs
files? My focus was only on files that contain secrets.
Applying this to all files increases the risk of breaking existing
userspace tooling.

Thanks,
//richard

-- 
​​​​​sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT UID/VAT Nr:
ATU 66964118 | FN: 374287y

Re: [PATCH 2/2] nvmet: Restrict in-band config files to root

[Sagi Grimberg]

Not sure who uses nvmet outside of root user, but looks fine to me:

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

Re: [PATCH 1/2] configfs: Add CONFIGFS_ATTR_PERM helper

[Chaitanya Kulkarni]

On 4/20/25 03:47, Richard Weinberger wrote:
> This new helper allows creating rw files with custom
> permissions.
>
> Signed-off-by: Richard Weinberger<richard@nod.at>


Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 1/2] configfs: Add CONFIGFS_ATTR_PERM helper

[Sagi Grimberg]

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

Re: [PATCH 1/2] configfs: Add CONFIGFS_ATTR_PERM helper

[Andreas Hindborg]

On Sun, 20 Apr 2025 12:47:25 +0200, Richard Weinberger wrote:
> This new helper allows creating rw files with custom
> permissions.
> 
> 

Applied, thanks!

[1/2] configfs: Add CONFIGFS_ATTR_PERM helper
      commit: 052ddfe405efcad262278301abf809e31f08605c

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>