All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Smack transmute fixes
@ 2023-07-24 15:13 Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

The first two patches are obvious fixes, the first restricts setting the
SMACK64TRANSMUTE xattr only for directories, and the second makes it
possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
(e.g. ramfs).

The remaining fixes are optional, and only required if we want filesystems
without xattr support behave like those with xattr support. Since we have
the inode_setsecurity and inode_getsecurity hooks to make the first group
work, it seems useful to fix inode creation too (SELinux should be fine).

The third patch is merely a code move out of the 'if (xattr)' condition.
The fourth updates the security field of the in-memory inode directly in
smack_inode_init_security() and marks the inode as instantiated, and the
fifth adds a security_inode_init_security() call in ramfs to initialize the
security field of the in-memory inodes (needed to test transmuting
directories).

Both the Smack (on xfs) and IMA test suite succeed with all patches
applied.

By executing the tests in a ramfs, the results are:

Without the patches:
86 Passed, 9 Failed, 90% Success rate

With the patches:
93 Passed, 2 Failed, 97% Success rate

The remaining two failures are:
2151  ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
2152  lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)

The first one is likely due ramfs lack of support for ioctl() while the
second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().

The patch set applies on top of:

https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/commit/?h=next&id=ca22eca6e2ad7eaed1c791628ef7cb4c739e3da6


The ramfs patch potentially could be useful to correctly initialize the
label of new inodes in the initramfs, assuming that it will be fully
labeled with support for xattrs in the cpio image:

https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/

Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.

Changelog

v1:
- Rebase on top of latest lsm/next
- Remove -EOPNOTSUPP check in patch 5 (cannot happen)

Roberto Sassu (5):
  smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
  smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
  smack: Always determine inode labels in smack_inode_init_security()
  smack: Initialize the in-memory inode in smack_inode_init_security()
  ramfs: Initialize security of in-memory inodes

 fs/ramfs/inode.c           | 27 +++++++++++
 security/smack/smack_lsm.c | 97 ++++++++++++++++++++++----------------
 2 files changed, 83 insertions(+), 41 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
@ 2023-07-24 15:13 ` Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity() Roberto Sassu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu, stable

From: Roberto Sassu <roberto.sassu@huawei.com>

Since the SMACK64TRANSMUTE xattr makes sense only for directories, enforce
this restriction in smack_inode_setxattr().

Cc: stable@vger.kernel.org
Fixes: 5c6d1125f8db ("Smack: Transmute labels on specified directories") # v2.6.38.x
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/smack/smack_lsm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 679156601a1..e599ce9453c 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1262,7 +1262,8 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
 		check_star = 1;
 	} else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
 		check_priv = 1;
-		if (size != TRANS_TRUE_SIZE ||
+		if (!S_ISDIR(d_backing_inode(dentry)->i_mode) ||
+		    size != TRANS_TRUE_SIZE ||
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
 			rc = -EINVAL;
 	} else
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
@ 2023-07-24 15:13 ` Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 3/5] smack: Always determine inode labels in smack_inode_init_security() Roberto Sassu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu, stable

From: Roberto Sassu <roberto.sassu@huawei.com>

If the SMACK64TRANSMUTE xattr is provided, and the inode is a directory,
update the in-memory inode flags by setting SMK_INODE_TRANSMUTE.

Cc: stable@vger.kernel.org
Fixes: 5c6d1125f8db ("Smack: Transmute labels on specified directories") # v2.6.38.x
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/smack/smack_lsm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e599ce9453c..9eae830527d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2804,6 +2804,15 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	if (value == NULL || size > SMK_LONGLABEL || size == 0)
 		return -EINVAL;
 
+	if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) {
+		if (!S_ISDIR(inode->i_mode) || size != TRANS_TRUE_SIZE ||
+		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
+			return -EINVAL;
+
+		nsp->smk_flags |= SMK_INODE_TRANSMUTE;
+		return 0;
+	}
+
 	skp = smk_import_entry(value, size);
 	if (IS_ERR(skp))
 		return PTR_ERR(skp);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] smack: Always determine inode labels in smack_inode_init_security()
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity() Roberto Sassu
@ 2023-07-24 15:13 ` Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 4/5] smack: Initialize the in-memory inode " Roberto Sassu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

The inode_init_security hook is already a good place to initialize the
in-memory inode. And that is also what SELinux does.

In preparation for this, move the existing smack_inode_init_security() code
outside the 'if (xattr)' condition, and set the xattr, if provided.

This change does not have any impact on the current code, since every time
security_inode_init_security() is called, the initxattr() callback is
passed and, thus, xattr is non-NULL.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/smack/smack_lsm.c | 80 +++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9eae830527d..5a31d005c6d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -948,51 +948,51 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 	struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count);
 	int may;
 
-	if (xattr) {
-		/*
-		 * If equal, transmuting already occurred in
-		 * smack_dentry_create_files_as(). No need to check again.
-		 */
-		if (tsp->smk_task != tsp->smk_transmuted) {
-			rcu_read_lock();
-			may = smk_access_entry(skp->smk_known, dsp->smk_known,
-					       &skp->smk_rules);
-			rcu_read_unlock();
-		}
+	/*
+	 * If equal, transmuting already occurred in
+	 * smack_dentry_create_files_as(). No need to check again.
+	 */
+	if (tsp->smk_task != tsp->smk_transmuted) {
+		rcu_read_lock();
+		may = smk_access_entry(skp->smk_known, dsp->smk_known,
+				       &skp->smk_rules);
+		rcu_read_unlock();
+	}
+
+	/*
+	 * In addition to having smk_task equal to smk_transmuted,
+	 * if the access rule allows transmutation and the directory
+	 * requests transmutation then by all means transmute.
+	 * Mark the inode as changed.
+	 */
+	if ((tsp->smk_task == tsp->smk_transmuted) ||
+	    (may > 0 && ((may & MAY_TRANSMUTE) != 0) &&
+	     smk_inode_transmutable(dir))) {
+		struct xattr *xattr_transmute;
 
 		/*
-		 * In addition to having smk_task equal to smk_transmuted,
-		 * if the access rule allows transmutation and the directory
-		 * requests transmutation then by all means transmute.
-		 * Mark the inode as changed.
+		 * The caller of smack_dentry_create_files_as()
+		 * should have overridden the current cred, so the
+		 * inode label was already set correctly in
+		 * smack_inode_alloc_security().
 		 */
-		if ((tsp->smk_task == tsp->smk_transmuted) ||
-		    (may > 0 && ((may & MAY_TRANSMUTE) != 0) &&
-		     smk_inode_transmutable(dir))) {
-			struct xattr *xattr_transmute;
-
-			/*
-			 * The caller of smack_dentry_create_files_as()
-			 * should have overridden the current cred, so the
-			 * inode label was already set correctly in
-			 * smack_inode_alloc_security().
-			 */
-			if (tsp->smk_task != tsp->smk_transmuted)
-				isp = dsp;
-			xattr_transmute = lsm_get_xattr_slot(xattrs,
-							     xattr_count);
-			if (xattr_transmute) {
-				xattr_transmute->value = kmemdup(TRANS_TRUE,
-								 TRANS_TRUE_SIZE,
-								 GFP_NOFS);
-				if (!xattr_transmute->value)
-					return -ENOMEM;
-
-				xattr_transmute->value_len = TRANS_TRUE_SIZE;
-				xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
-			}
+		if (tsp->smk_task != tsp->smk_transmuted)
+			isp = dsp;
+		xattr_transmute = lsm_get_xattr_slot(xattrs,
+						     xattr_count);
+		if (xattr_transmute) {
+			xattr_transmute->value = kmemdup(TRANS_TRUE,
+							 TRANS_TRUE_SIZE,
+							 GFP_NOFS);
+			if (!xattr_transmute->value)
+				return -ENOMEM;
+
+			xattr_transmute->value_len = TRANS_TRUE_SIZE;
+			xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
 		}
+	}
 
+	if (xattr) {
 		xattr->value = kstrdup(isp->smk_known, GFP_NOFS);
 		if (!xattr->value)
 			return -ENOMEM;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] smack: Initialize the in-memory inode in smack_inode_init_security()
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
                   ` (2 preceding siblings ...)
  2023-07-24 15:13 ` [PATCH v2 3/5] smack: Always determine inode labels in smack_inode_init_security() Roberto Sassu
@ 2023-07-24 15:13 ` Roberto Sassu
  2023-07-24 15:13 ` [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
  2023-07-25 18:18 ` [PATCH v2 0/5] Smack transmute fixes Casey Schaufler
  5 siblings, 0 replies; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

Currently, Smack initializes in-memory new inodes in three steps. It first
sets the xattrs in smack_inode_init_security(), fetches them in
smack_d_instantiate() and finally, in the same function, sets the in-memory
inodes depending on xattr values, unless they are in specially-handled
filesystems.

Other than being inefficient, this also prevents filesystems not supporting
xattrs from working properly since, without xattrs, there is no way to pass
the label determined in smack_inode_init_security() to
smack_d_instantiate().

Since the LSM infrastructure allows setting and getting the security field
without xattrs through the inode_setsecurity and inode_getsecurity hooks,
make the inode creation work too, by initializing the in-memory inode
earlier in smack_inode_init_security().

Also mark the inode as instantiated, to prevent smack_d_instantiate() from
overwriting the security field. As mentioned above, this potentially has
impact for inodes in specially-handled filesystems in
smack_d_instantiate(), if they are not handled in the same way in
smack_inode_init_security().

Filesystems other than tmpfs don't call security_inode_init_security(), so
they would be always initialized in smack_d_instantiate(), as before. For
tmpfs, the current behavior is to assign to inodes the label '*', but
actually that label is overwritten with the one fetched from the SMACK64
xattr, set in smack_inode_init_security() (default: '_').

Initializing the in-memory inode is straightforward: if not transmuting,
nothing more needs to be done; if transmuting, overwrite the current inode
label with the one from the parent directory, and set SMK_INODE_TRANSMUTE.
Finally, set SMK_INODE_INSTANT for all cases, to mark the inode as
instantiated.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/smack/smack_lsm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5a31d005c6d..f3946778192 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -942,6 +942,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 				     struct xattr *xattrs, int *xattr_count)
 {
 	struct task_smack *tsp = smack_cred(current_cred());
+	struct inode_smack *issp = smack_inode(inode);
 	struct smack_known *skp = smk_of_task(tsp);
 	struct smack_known *isp = smk_of_inode(inode);
 	struct smack_known *dsp = smk_of_inode(dir);
@@ -977,7 +978,9 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 		 * smack_inode_alloc_security().
 		 */
 		if (tsp->smk_task != tsp->smk_transmuted)
-			isp = dsp;
+			isp = issp->smk_inode = dsp;
+
+		issp->smk_flags |= SMK_INODE_TRANSMUTE;
 		xattr_transmute = lsm_get_xattr_slot(xattrs,
 						     xattr_count);
 		if (xattr_transmute) {
@@ -992,6 +995,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
 		}
 	}
 
+	issp->smk_flags |= SMK_INODE_INSTANT;
+
 	if (xattr) {
 		xattr->value = kstrdup(isp->smk_known, GFP_NOFS);
 		if (!xattr->value)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
                   ` (3 preceding siblings ...)
  2023-07-24 15:13 ` [PATCH v2 4/5] smack: Initialize the in-memory inode " Roberto Sassu
@ 2023-07-24 15:13 ` Roberto Sassu
  2023-11-15  8:01   ` Roberto Sassu
  2023-07-25 18:18 ` [PATCH v2 0/5] Smack transmute fixes Casey Schaufler
  5 siblings, 1 reply; 9+ messages in thread
From: Roberto Sassu @ 2023-07-24 15:13 UTC (permalink / raw)
  To: casey, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu

From: Roberto Sassu <roberto.sassu@huawei.com>

Add a call security_inode_init_security() after ramfs_get_inode(), to let
LSMs initialize the inode security field. Skip ramfs_fill_super(), as the
initialization is done through the sb_set_mnt_opts hook.

Calling security_inode_init_security() call inside ramfs_get_inode() is
not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after
the latter.

Pass NULL as initxattrs() callback to security_inode_init_security(), since
the purpose of the call is only to initialize the in-memory inodes.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/ramfs/inode.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index fef477c7810..ac90ebd9dbd 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 	int error = -ENOSPC;
 
 	if (inode) {
+		error = security_inode_init_security(inode, dir,
+						     &dentry->d_name, NULL,
+						     NULL);
+		if (error) {
+			iput(inode);
+			return error;
+		}
+
 		d_instantiate(dentry, inode);
 		dget(dentry);	/* Extra count - pin the dentry in core */
 		error = 0;
@@ -134,6 +142,15 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0);
 	if (inode) {
 		int l = strlen(symname)+1;
+
+		error = security_inode_init_security(inode, dir,
+						     &dentry->d_name, NULL,
+						     NULL);
+		if (error) {
+			iput(inode);
+			return error;
+		}
+
 		error = page_symlink(inode, symname, l);
 		if (!error) {
 			d_instantiate(dentry, inode);
@@ -149,10 +166,20 @@ static int ramfs_tmpfile(struct mnt_idmap *idmap,
 			 struct inode *dir, struct file *file, umode_t mode)
 {
 	struct inode *inode;
+	int error;
 
 	inode = ramfs_get_inode(dir->i_sb, dir, mode, 0);
 	if (!inode)
 		return -ENOSPC;
+
+	error = security_inode_init_security(inode, dir,
+					     &file_dentry(file)->d_name, NULL,
+					     NULL);
+	if (error) {
+		iput(inode);
+		return error;
+	}
+
 	d_tmpfile(file, inode);
 	return finish_open_simple(file, 0);
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] Smack transmute fixes
  2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
                   ` (4 preceding siblings ...)
  2023-07-24 15:13 ` [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
@ 2023-07-25 18:18 ` Casey Schaufler
  5 siblings, 0 replies; 9+ messages in thread
From: Casey Schaufler @ 2023-07-25 18:18 UTC (permalink / raw)
  To: Roberto Sassu, paul, jmorris, serge
  Cc: linux-kernel, linux-security-module, Roberto Sassu, Casey Schaufler

On 7/24/2023 8:13 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>

I'm testing these relative to smack-next.

>
> The first two patches are obvious fixes, the first restricts setting the
> SMACK64TRANSMUTE xattr only for directories, and the second makes it
> possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
> (e.g. ramfs).
>
> The remaining fixes are optional, and only required if we want filesystems
> without xattr support behave like those with xattr support. Since we have
> the inode_setsecurity and inode_getsecurity hooks to make the first group
> work, it seems useful to fix inode creation too (SELinux should be fine).
>
> The third patch is merely a code move out of the 'if (xattr)' condition.
> The fourth updates the security field of the in-memory inode directly in
> smack_inode_init_security() and marks the inode as instantiated, and the
> fifth adds a security_inode_init_security() call in ramfs to initialize the
> security field of the in-memory inodes (needed to test transmuting
> directories).
>
> Both the Smack (on xfs) and IMA test suite succeed with all patches
> applied.
>
> By executing the tests in a ramfs, the results are:
>
> Without the patches:
> 86 Passed, 9 Failed, 90% Success rate
>
> With the patches:
> 93 Passed, 2 Failed, 97% Success rate
>
> The remaining two failures are:
> 2151  ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
> 2152  lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)
>
> The first one is likely due ramfs lack of support for ioctl() while the
> second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().
>
> The patch set applies on top of:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/commit/?h=next&id=ca22eca6e2ad7eaed1c791628ef7cb4c739e3da6
>
>
> The ramfs patch potentially could be useful to correctly initialize the
> label of new inodes in the initramfs, assuming that it will be fully
> labeled with support for xattrs in the cpio image:
>
> https://lore.kernel.org/linux-integrity/20190523121803.21638-1-roberto.sassu@huawei.com/
>
> Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.
>
> Changelog
>
> v1:
> - Rebase on top of latest lsm/next
> - Remove -EOPNOTSUPP check in patch 5 (cannot happen)
>
> Roberto Sassu (5):
>   smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
>   smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
>   smack: Always determine inode labels in smack_inode_init_security()
>   smack: Initialize the in-memory inode in smack_inode_init_security()
>   ramfs: Initialize security of in-memory inodes
>
>  fs/ramfs/inode.c           | 27 +++++++++++
>  security/smack/smack_lsm.c | 97 ++++++++++++++++++++++----------------
>  2 files changed, 83 insertions(+), 41 deletions(-)
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes
  2023-07-24 15:13 ` [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
@ 2023-11-15  8:01   ` Roberto Sassu
  2023-11-15 22:24     ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Roberto Sassu @ 2023-11-15  8:01 UTC (permalink / raw)
  To: casey, paul, jmorris, serge, Andrew Morton
  Cc: linux-kernel, linux-security-module, Roberto Sassu

On Mon, 2023-07-24 at 17:13 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Add a call security_inode_init_security() after ramfs_get_inode(), to let
> LSMs initialize the inode security field. Skip ramfs_fill_super(), as the
> initialization is done through the sb_set_mnt_opts hook.
> 
> Calling security_inode_init_security() call inside ramfs_get_inode() is
> not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after
> the latter.
> 
> Pass NULL as initxattrs() callback to security_inode_init_security(), since
> the purpose of the call is only to initialize the in-memory inodes.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

+ Andrew

Hi Andrew

I'm proposing an extension to initialize the inode security field at
inode creation time for filesystems that don't support xattrs (ramfs in
this case).

The LSM infrastructure already supports setting the inode security
field, but only at run-time, with the inode_setsecurity hook.

I developed this to do some testing on the Smack LSM, and I thought it
could be useful anyway.

Casey would need your acked-by, to carry this patch in his repository.
I'm not completely sure if you are the maintainer, but in the past you
accepted a patch for ramfs.

If you have time and you could have a look, that would be great!

Thanks

Roberto

> ---
>  fs/ramfs/inode.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index fef477c7810..ac90ebd9dbd 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
>  	int error = -ENOSPC;
>  
>  	if (inode) {
> +		error = security_inode_init_security(inode, dir,
> +						     &dentry->d_name, NULL,
> +						     NULL);
> +		if (error) {
> +			iput(inode);
> +			return error;
> +		}
> +
>  		d_instantiate(dentry, inode);
>  		dget(dentry);	/* Extra count - pin the dentry in core */
>  		error = 0;
> @@ -134,6 +142,15 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  	inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0);
>  	if (inode) {
>  		int l = strlen(symname)+1;
> +
> +		error = security_inode_init_security(inode, dir,
> +						     &dentry->d_name, NULL,
> +						     NULL);
> +		if (error) {
> +			iput(inode);
> +			return error;
> +		}
> +
>  		error = page_symlink(inode, symname, l);
>  		if (!error) {
>  			d_instantiate(dentry, inode);
> @@ -149,10 +166,20 @@ static int ramfs_tmpfile(struct mnt_idmap *idmap,
>  			 struct inode *dir, struct file *file, umode_t mode)
>  {
>  	struct inode *inode;
> +	int error;
>  
>  	inode = ramfs_get_inode(dir->i_sb, dir, mode, 0);
>  	if (!inode)
>  		return -ENOSPC;
> +
> +	error = security_inode_init_security(inode, dir,
> +					     &file_dentry(file)->d_name, NULL,
> +					     NULL);
> +	if (error) {
> +		iput(inode);
> +		return error;
> +	}
> +
>  	d_tmpfile(file, inode);
>  	return finish_open_simple(file, 0);
>  }


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes
  2023-11-15  8:01   ` Roberto Sassu
@ 2023-11-15 22:24     ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2023-11-15 22:24 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: casey, paul, jmorris, serge, linux-kernel, linux-security-module,
	Roberto Sassu, Hugh Dickins

On Wed, 15 Nov 2023 09:01:52 +0100 Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:

> On Mon, 2023-07-24 at 17:13 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Add a call security_inode_init_security() after ramfs_get_inode(), to let
> > LSMs initialize the inode security field. Skip ramfs_fill_super(), as the
> > initialization is done through the sb_set_mnt_opts hook.
> > 
> > Calling security_inode_init_security() call inside ramfs_get_inode() is
> > not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after
> > the latter.
> > 
> > Pass NULL as initxattrs() callback to security_inode_init_security(), since
> > the purpose of the call is only to initialize the in-memory inodes.
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> + Andrew
> 
> Hi Andrew
> 
> I'm proposing an extension to initialize the inode security field at
> inode creation time for filesystems that don't support xattrs (ramfs in
> this case).
> 
> The LSM infrastructure already supports setting the inode security
> field, but only at run-time, with the inode_setsecurity hook.
> 
> I developed this to do some testing on the Smack LSM, and I thought it
> could be useful anyway.
> 
> Casey would need your acked-by, to carry this patch in his repository.
> I'm not completely sure if you are the maintainer, but in the past you
> accepted a patch for ramfs.
> 
> If you have time and you could have a look, that would be great!

Patch looks OK to me.  Please cc Hugh and myself on a resend.

One little thing:

> > +++ b/fs/ramfs/inode.c
> > @@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> >  	int error = -ENOSPC;
> >  
> >  	if (inode) {
> > +		error = security_inode_init_security(inode, dir,
> > +						     &dentry->d_name, NULL,
> > +						     NULL);
> > +		if (error) {
> > +			iput(inode);
> > +			return error;

A `break' here would be better.  To avoid having multiple return
points, which are often a maintenance hassle.  Same treatment at
the other sites.



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-11-15 22:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-24 15:13 [PATCH v2 0/5] Smack transmute fixes Roberto Sassu
2023-07-24 15:13 ` [PATCH v2 1/5] smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr() Roberto Sassu
2023-07-24 15:13 ` [PATCH v2 2/5] smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity() Roberto Sassu
2023-07-24 15:13 ` [PATCH v2 3/5] smack: Always determine inode labels in smack_inode_init_security() Roberto Sassu
2023-07-24 15:13 ` [PATCH v2 4/5] smack: Initialize the in-memory inode " Roberto Sassu
2023-07-24 15:13 ` [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes Roberto Sassu
2023-11-15  8:01   ` Roberto Sassu
2023-11-15 22:24     ` Andrew Morton
2023-07-25 18:18 ` [PATCH v2 0/5] Smack transmute fixes Casey Schaufler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.