linux-kernel - [PATCH v2 3/4] ima: check appraisal flag in the ima_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed,  1 Oct 2014 21:43:09 +0300
From:	Dmitry Kasatkin <dmitry.kasatkin@...il.com>
To:	zohar@...ux.vnet.ibm.com, linux-ima-devel@...ts.sourceforge.net,
	linux-security-module@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org,
	Dmitry Kasatkin <d.kasatkin@...sung.com>
Subject: [PATCH v2 3/4] ima: check appraisal flag in the ima_file_free() hook

ima_file_free() hook is only used by appraisal module to update hash
when file was modified. When there were no integrity checks on inode,
S_IMA flag is not set, integrity_iint_find() returns NULL and it
quits. When inode is only measured/audited but not appraised, iint
is allocated and it will cause calling ima_check_last_writer() which
unnecessarily locks i_mutex.

Currently ima_file_free() checks 'iint_initialized'. But it looks that
it is a mistake and originally 'ima_appraise' had to be used instead.
In fact usage of 'iint_initialized' is completely unnecessary because
S_IMA flag would not be set if iint was not allocated.

This patch uses lately introduced ima_policy_flag to test if appraisal
was enabled by policy.

With this change 'iint_initialized' is no longer used anywhere. Indeed,
integrity cache is allocated with SLAB_PANIC and kernel will panic if
allocation fails during kernel initialization. So this variable is
unnecessary and thus this patch removes it.

Changes in v2:
* 'iint_initialized' removal patch merged to this patch (requested
   by Mimi)

Signed-off-by: Dmitry Kasatkin <d.kasatkin@...sung.com>
---
 security/integrity/iint.c         | 3 ---
 security/integrity/ima/ima_main.c | 2 +-
 security/integrity/integrity.h    | 3 ---
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index a521edf..cc3eb4d 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -25,8 +25,6 @@ static struct rb_root integrity_iint_tree = RB_ROOT;
 static DEFINE_RWLOCK(integrity_iint_lock);
 static struct kmem_cache *iint_cache __read_mostly;
 
-int iint_initialized;
-
 /*
  * __integrity_iint_find - return the iint associated with an inode
  */
@@ -166,7 +164,6 @@ static int __init integrity_iintcache_init(void)
 	iint_cache =
 	    kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
 			      0, SLAB_PANIC, init_once);
-	iint_initialized = 1;
 	return 0;
 }
 security_initcall(integrity_iintcache_init);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 62f59ec..87d7df7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -143,7 +143,7 @@ void ima_file_free(struct file *file)
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint;
 
-	if (!iint_initialized || !S_ISREG(inode->i_mode))
+	if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode))
 		return;
 
 	iint = integrity_iint_find(inode);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index aafb468..f51ad65 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -169,6 +169,3 @@ static inline void integrity_audit_msg(int audit_msgno, struct inode *inode,
 {
 }
 #endif
-
-/* set during initialization */
-extern int iint_initialized;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/