diff mbox series

[v3,1/6] fs: ima: Remove S_IMA and IS_IMA()

Message ID 20250122172432.3074180-2-roberto.sassu@huaweicloud.com (mailing list archive)
State Handled Elsewhere
Headers show
Series ima: Remove unnecessary inode locks | expand

Commit Message

Roberto Sassu Jan. 22, 2025, 5:24 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

Commit 196f518128d2e ("IMA: explicit IMA i_flag to remove global lock on
inode_delete") introduced the new S_IMA inode flag to determine whether or
not an inode was processed by IMA. In that way, it was not necessary to
take the global lock on inode delete.

Since commit 4de2f084fbff ("ima: Make it independent from 'integrity'
LSM"), the pointer of the inode integrity metadata managed by IMA has been
moved to the inode security blob, from the rb-tree. The pointer is not NULL
only if the inode has been processed by IMA, i.e. ima_inode_get() has been
called for that inode.

Thus, since the IS_IMA() check can be now implemented by trivially testing
whether or not the pointer of inode integrity metadata is NULL, remove the
S_IMA definition in include/linux/fs.h and also the IS_IMA() macro.

Remove also the IS_IMA() invocation in ima_rdwr_violation_check(), since
whether the inode was processed by IMA will be anyway detected by a
subsequent call to ima_iint_find(). It does not have an additional overhead
since the decision can be made in constant time, as opposed to logarithm
when the inode integrity metadata was stored in the rb-tree.

Suggested-by: Shu Han <ebpqwerty472123@gmail.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Acked-by: Jan Kara <jack@suse.cz>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/fs.h                | 2 --
 security/integrity/ima/ima_iint.c | 5 -----
 security/integrity/ima/ima_main.c | 2 +-
 3 files changed, 1 insertion(+), 8 deletions(-)

Comments

Mimi Zohar Jan. 31, 2025, 4:54 p.m. UTC | #1 
On Wed, 2025-01-22 at 18:24 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Commit 196f518128d2e ("IMA: explicit IMA i_flag to remove global lock on
> inode_delete") introduced the new S_IMA inode flag to determine whether or
> not an inode was processed by IMA. In that way, it was not necessary to
> take the global lock on inode delete.
> 
> Since commit 4de2f084fbff ("ima: Make it independent from 'integrity'
> LSM"), the pointer of the inode integrity metadata managed by IMA has been
> moved to the inode security blob, from the rb-tree. The pointer is not NULL
> only if the inode has been processed by IMA, i.e. ima_inode_get() has been
> called for that inode.
> 
> Thus, since the IS_IMA() check can be now implemented by trivially testing
> whether or not the pointer of inode integrity metadata is NULL, remove the
> S_IMA definition in include/linux/fs.h and also the IS_IMA() macro.
> 
> Remove also the IS_IMA() invocation in ima_rdwr_violation_check(), since
> whether the inode was processed by IMA will be anyway detected by a
> subsequent call to ima_iint_find(). It does not have an additional overhead
> since the decision can be made in constant time, as opposed to logarithm
> when the inode integrity metadata was stored in the rb-tree.
> 
> Suggested-by: Shu Han <ebpqwerty472123@gmail.com>
> Reviewed-by: Christian Brauner <brauner@kernel.org>
> Acked-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviewd-by: Mimi Zohar <zohar@linux.ibm.com>
diff mbox series

Patch

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7e29433c5ecc..8ee6961ab54a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2272,7 +2272,6 @@  struct super_operations {
 #define S_NOCMTIME	(1 << 7)  /* Do not update file c/mtime */
 #define S_SWAPFILE	(1 << 8)  /* Do not truncate: swapon got its bmaps */
 #define S_PRIVATE	(1 << 9)  /* Inode is fs-internal */
-#define S_IMA		(1 << 10) /* Inode has an associated IMA struct */
 #define S_AUTOMOUNT	(1 << 11) /* Automount/referral quasi-directory */
 #define S_NOSEC		(1 << 12) /* no suid or xattr security attributes */
 #ifdef CONFIG_FS_DAX
@@ -2330,7 +2329,6 @@  static inline bool sb_rdonly(const struct super_block *sb) { return sb->s_flags
 #endif
 
 #define IS_PRIVATE(inode)	((inode)->i_flags & S_PRIVATE)
-#define IS_IMA(inode)		((inode)->i_flags & S_IMA)
 #define IS_AUTOMOUNT(inode)	((inode)->i_flags & S_AUTOMOUNT)
 #define IS_NOSEC(inode)		((inode)->i_flags & S_NOSEC)
 #define IS_DAX(inode)		((inode)->i_flags & S_DAX)
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index 00b249101f98..9d9fc7a911ad 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -26,9 +26,6 @@  static struct kmem_cache *ima_iint_cache __ro_after_init;
  */
 struct ima_iint_cache *ima_iint_find(struct inode *inode)
 {
-	if (!IS_IMA(inode))
-		return NULL;
-
 	return ima_inode_get_iint(inode);
 }
 
@@ -102,7 +99,6 @@  struct ima_iint_cache *ima_inode_get(struct inode *inode)
 
 	ima_iint_init_always(iint, inode);
 
-	inode->i_flags |= S_IMA;
 	ima_inode_set_iint(inode, iint);
 
 	return iint;
@@ -118,7 +114,6 @@  void ima_inode_free_rcu(void *inode_security)
 {
 	struct ima_iint_cache **iint_p = inode_security + ima_blob_sizes.lbs_inode;
 
-	/* *iint_p should be NULL if !IS_IMA(inode) */
 	if (*iint_p)
 		ima_iint_free(*iint_p);
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 9b87556b03a7..6551be5754de 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -126,7 +126,7 @@  static void ima_rdwr_violation_check(struct file *file,
 	bool send_tomtou = false, send_writers = false;
 
 	if (mode & FMODE_WRITE) {
-		if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
+		if (atomic_read(&inode->i_readcount)) {
 			if (!iint)
 				iint = ima_iint_find(inode);
 			/* IMA_MEASURE is set from reader side */