[PATCH 0/8] integrity: fixes and new features

[PATCH 0/8] integrity: fixes and new features

[Dmitry Kasatkin]

Hi,

This patchset contains bug fixes, cleanups and new features
for integrity subsytem.

- Dmitry

Dmitry Kasatkin (8):
  ima: fix erronous removal of security.ima xattr
  integrity: fix checkpatch errors
  ima: return d_name.name if d_path fails
  evm: EVM does not use MD5
  ima: skip memory allocation for empty files
  evm: enable key retention service automatically
  evm: introduce EVM hmac attribute list
  evm: introduce EVM hmac xattr list

 security/integrity/evm/Kconfig        | 35 +++++++++-----
 security/integrity/evm/evm.h          | 41 +++++++++-------
 security/integrity/evm/evm_crypto.c   | 14 +++---
 security/integrity/evm/evm_main.c     | 90 +++++++++++++++++++++--------------
 security/integrity/iint.c             |  2 +-
 security/integrity/ima/ima_api.c      | 10 ++--
 security/integrity/ima/ima_crypto.c   | 22 +++++----
 security/integrity/ima/ima_fs.c       |  6 +--
 security/integrity/ima/ima_main.c     | 11 ++---
 security/integrity/ima/ima_policy.c   | 75 +++++++++++++++--------------
 security/integrity/ima/ima_queue.c    |  4 +-
 security/integrity/ima/ima_template.c | 14 +++---
 security/integrity/integrity_audit.c  |  4 +-
 13 files changed, 183 insertions(+), 145 deletions(-)

-- 
1.8.3.2

[PATCH 1/8] ima: fix erronous removal of security.ima xattr

[Dmitry Kasatkin]

ima_inode_post_setattr() calls ima_must_appraise() to check if
file needs to be appraised. If it is not then it removes security.ima
xattr. With original policy matching code it might happen that even
file needs to be appraised with FILE_CHECK hook, it might not be
for POST_SETATTR hook. 'security.ima' might be erronously removed.

This patch treats POST_SETATTR as special wildcard function and will
cause ima_must_appraise() to be true if any of the hooks rules matches.
security.ima will not be removed if any of the hooks would require
appraisal.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_policy.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d7c97a4..947cdbe 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -167,9 +167,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
 	const struct cred *cred = current_cred();
 	int i;
 
-	if ((rule->flags & IMA_FUNC) && rule->func != func)
+	if ((rule->flags & IMA_FUNC) &&
+		    (rule->func != func && func != POST_SETATTR))
 		return false;
-	if ((rule->flags & IMA_MASK) && rule->mask != mask)
+	if ((rule->flags & IMA_MASK) &&
+		    (rule->mask != mask && func != POST_SETATTR))
 		return false;
 	if ((rule->flags & IMA_FSMAGIC)
 	    && rule->fsmagic != inode->i_sb->s_magic)
-- 
1.8.3.2

[PATCH 2/8] integrity: fix checkpatch errors

[Dmitry Kasatkin]

Unfixed checkpatch errors make it difficult to see new errors..
This patch fix them.
Some lines with over 80 chars remained unchanged to improve
code readability.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/evm.h          | 28 +++++++-------
 security/integrity/evm/evm_crypto.c   |  4 +-
 security/integrity/iint.c             |  2 +-
 security/integrity/ima/ima_api.c      |  8 ++--
 security/integrity/ima/ima_crypto.c   |  2 +-
 security/integrity/ima/ima_fs.c       |  6 +--
 security/integrity/ima/ima_main.c     |  4 +-
 security/integrity/ima/ima_policy.c   | 69 +++++++++++++++++------------------
 security/integrity/ima/ima_queue.c    |  4 +-
 security/integrity/ima/ima_template.c | 14 +++----
 security/integrity/integrity_audit.c  |  4 +-
 11 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 30bd1ec..37c88dd 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
-extern int evm_init_key(void);
-extern int evm_update_evmxattr(struct dentry *dentry,
-			       const char *req_xattr_name,
-			       const char *req_xattr_value,
-			       size_t req_xattr_value_len);
-extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
-			 const char *req_xattr_value,
-			 size_t req_xattr_value_len, char *digest);
-extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
-			 const char *req_xattr_value,
-			 size_t req_xattr_value_len, char *digest);
-extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
-			 char *hmac_val);
-extern int evm_init_secfs(void);
+int evm_init_key(void);
+int evm_update_evmxattr(struct dentry *dentry,
+			const char *req_xattr_name,
+			const char *req_xattr_value,
+			size_t req_xattr_value_len);
+int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
+		  const char *req_xattr_value,
+		  size_t req_xattr_value_len, char *digest);
+int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
+		  const char *req_xattr_value,
+		  size_t req_xattr_value_len, char *digest);
+int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
+		  char *hmac_val);
+int evm_init_secfs(void);
 
 #endif
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 9bd329f..babd862 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 		umode_t mode;
 	} hmac_misc;
 
-	memset(&hmac_misc, 0, sizeof hmac_misc);
+	memset(&hmac_misc, 0, sizeof(hmac_misc));
 	hmac_misc.ino = inode->i_ino;
 	hmac_misc.generation = inode->i_generation;
 	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
 	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
 	hmac_misc.mode = inode->i_mode;
-	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
+	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
 	if (evm_hmac_version > 1)
 		crypto_shash_update(desc, inode->i_sb->s_uuid,
 				    sizeof(inode->i_sb->s_uuid));
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c49d3f1..a521edf 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -151,7 +151,7 @@ static void init_once(void *foo)
 {
 	struct integrity_iint_cache *iint = foo;
 
-	memset(iint, 0, sizeof *iint);
+	memset(iint, 0, sizeof(*iint));
 	iint->version = 0;
 	iint->flags = 0UL;
 	iint->ima_file_status = INTEGRITY_UNKNOWN;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 393b9d4..c6b4a73 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -160,10 +160,10 @@ err_out:
  * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK)
  *
  * The policy is defined in terms of keypairs:
- * 		subj=, obj=, type=, func=, mask=, fsmagic=
+ *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
- * 	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
- * 	mask: contains the permission mask
+ *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
+ *	mask: contains the permission mask
  *	fsmagic: hex value
  *
  * Returns IMA_MEASURE, IMA_APPRAISE mask.
@@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
  *
  * We only get here if the inode has not already been measured,
  * but the measurement could already exist:
- * 	- multiple copies of the same file on either the same or
+ *	- multiple copies of the same file on either the same or
  *	  different filesystems.
  *	- the inode was previously flushed as well as the iint info,
  *	  containing the hashing info.
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 9999057..d257e36 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -10,7 +10,7 @@
  * the Free Software Foundation, version 2 of the License.
  *
  * File: ima_crypto.c
- * 	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
+ *	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 468a3ba..da92fcc 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v)
 	 * PCR used is always the same (config option) in
 	 * little-endian format
 	 */
-	ima_putc(m, &pcr, sizeof pcr);
+	ima_putc(m, &pcr, sizeof(pcr));
 
 	/* 2nd: template digest */
 	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3rd: template name size */
 	namelen = strlen(e->template_desc->name);
-	ima_putc(m, &namelen, sizeof namelen);
+	ima_putc(m, &namelen, sizeof(namelen));
 
 	/* 4th:  template name */
 	ima_putc(m, e->template_desc->name, namelen);
@@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1);
 /*
  * ima_open_policy: sequentialize access to the policy file
  */
-static int ima_open_policy(struct inode * inode, struct file * filp)
+static int ima_open_policy(struct inode *inode, struct file *filp)
 {
 	/* No point in being allowed to open it if you aren't going to write */
 	if (!(filp->f_flags & O_WRONLY))
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 149ee11..50413d0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup);
  * ima_rdwr_violation_check
  *
  * Only invalidate the PCR for measured files:
- * 	- Opening a file for write when already open for read,
+ *	- Opening a file for write when already open for read,
  *	  results in a time of measure, time of use (ToMToU) error.
  *	- Opening a file for read when already open for write,
- * 	  could result in a file measurement error.
+ *	  could result in a file measurement error.
  *
  */
 static void ima_rdwr_violation_check(struct file *file)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 947cdbe..41021b4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -7,7 +7,7 @@
  * the Free Software Foundation, version 2 of the License.
  *
  * ima_policy.c
- * 	- initialize default measure policy rules
+ *	- initialize default measure policy rules
  *
  */
 #include <linux/module.h>
@@ -21,8 +21,8 @@
 #include "ima.h"
 
 /* flags definitions */
-#define IMA_FUNC 	0x0001
-#define IMA_MASK 	0x0002
+#define IMA_FUNC	0x0001
+#define IMA_MASK	0x0002
 #define IMA_FSMAGIC	0x0004
 #define IMA_UID		0x0008
 #define IMA_FOWNER	0x0010
@@ -69,35 +69,35 @@ struct ima_rule_entry {
  * and running executables.
  */
 static struct ima_rule_entry default_rules[] = {
-	{.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC,
+	{.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
+	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID,
+	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID,
 	 .flags = IMA_FUNC | IMA_MASK | IMA_UID},
-	{.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_appraise_rules[] = {
-	{.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
-	{.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
+	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
 };
 
 static LIST_HEAD(ima_default_rules);
@@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str)
 }
 __setup("ima_appraise_tcb", default_appraise_policy_setup);
 
-/* 
+/*
  * Although the IMA policy does not change, the LSM policy can be
  * reloaded, leaving the IMA LSM based rules referring to the old,
  * stale LSM policy.
  *
- * Update the IMA LSM based rules to reflect the reloaded LSM policy. 
+ * Update the IMA LSM based rules to reflect the reloaded LSM policy.
  * We assume the rules still exist; and BUG_ON() if they don't.
  */
 static void ima_lsm_update_rules(void)
@@ -218,7 +218,7 @@ retry:
 			retried = 1;
 			ima_lsm_update_rules();
 			goto retry;
-		} 
+		}
 		if (!rc)
 			return false;
 	}
@@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 	if (!(rule->flags & IMA_FUNC))
 		return IMA_FILE_APPRAISE;
 
-	switch(func) {
+	switch (func) {
 	case MMAP_CHECK:
 		return IMA_MMAP_APPRAISE;
 	case BPRM_CHECK:
@@ -306,7 +306,7 @@ void __init ima_init_policy(void)
 	measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0;
 	appraise_entries = ima_use_appraise_tcb ?
 			 ARRAY_SIZE(default_appraise_rules) : 0;
-	
+
 	for (i = 0; i < measure_entries + appraise_entries; i++) {
 		if (i < measure_entries)
 			list_add_tail(&default_rules[i].list,
@@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				break;
 			}
 
-			result = strict_strtoul(args[0].from, 16,
-						&entry->fsmagic);
+			result = kstrtoul(args[0].from, 16, &entry->fsmagic);
 			if (!result)
 				entry->flags |= IMA_FSMAGIC;
 			break;
@@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				break;
 			}
 
-			result = strict_strtoul(args[0].from, 10, &lnum);
+			result = kstrtoul(args[0].from, 10, &lnum);
 			if (!result) {
 				entry->uid = make_kuid(current_user_ns(), (uid_t)lnum);
 				if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum))
@@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				break;
 			}
 
-			result = strict_strtoul(args[0].from, 10, &lnum);
+			result = kstrtoul(args[0].from, 10, &lnum);
 			if (!result) {
 				entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum);
 				if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum))
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 91128b4..552705d 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 
 	mutex_lock(&ima_extend_list_mutex);
 	if (!violation) {
-		memcpy(digest, entry->digest, sizeof digest);
+		memcpy(digest, entry->digest, sizeof(digest));
 		if (ima_lookup_digest_entry(digest)) {
 			audit_cause = "hash_exists";
 			result = -EEXIST;
@@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 	}
 
 	if (violation)		/* invalidate pcr */
-		memset(digest, 0xff, sizeof digest);
+		memset(digest, 0xff, sizeof(digest));
 
 	tpmresult = ima_pcr_extend(digest);
 	if (tpmresult != 0) {
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 9a4a0d1..a076a96 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -22,20 +22,20 @@
 
 static struct ima_template_desc defined_templates[] = {
 	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
-	{.name = "ima-ng",.fmt = "d-ng|n-ng"},
-	{.name = "ima-sig",.fmt = "d-ng|n-ng|sig"},
+	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
+	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 };
 
 static struct ima_template_field supported_fields[] = {
-	{.field_id = "d",.field_init = ima_eventdigest_init,
+	{.field_id = "d", .field_init = ima_eventdigest_init,
 	 .field_show = ima_show_template_digest},
-	{.field_id = "n",.field_init = ima_eventname_init,
+	{.field_id = "n", .field_init = ima_eventname_init,
 	 .field_show = ima_show_template_string},
-	{.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
+	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
 	 .field_show = ima_show_template_digest_ng},
-	{.field_id = "n-ng",.field_init = ima_eventname_ng_init,
+	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
 	 .field_show = ima_show_template_string},
-	{.field_id = "sig",.field_init = ima_eventsig_init,
+	{.field_id = "sig", .field_init = ima_eventsig_init,
 	 .field_show = ima_show_template_sig},
 };
 
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index 793d7be..aab9fa5 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -7,7 +7,7 @@
  * the Free Software Foundation, version 2 of the License.
  *
  * File: integrity_audit.c
- * 	Audit calls for the integrity subsystem
+ *	Audit calls for the integrity subsystem
  */
 
 #include <linux/fs.h>
@@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str)
 {
 	unsigned long audit;
 
-	if (!strict_strtoul(str, 0, &audit))
+	if (!kstrtoul(str, 0, &audit))
 		integrity_audit_info = audit ? 1 : 0;
 	return 1;
 }
-- 
1.8.3.2

[PATCH 3/8] ima: return d_name.name if d_path fails

[Dmitry Kasatkin]

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_api.c  | 2 +-
 security/integrity/ima/ima_main.c | 7 +------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c6b4a73..ba9e4d7 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -332,5 +332,5 @@ const char *ima_d_path(struct path *path, char **pathbuf)
 			pathname = NULL;
 		}
 	}
-	return pathname;
+	return pathname ?: (const char *)path->dentry->d_name.name;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 50413d0..52ac6cf 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -79,7 +79,6 @@ __setup("ima_hash=", hash_setup);
  */
 static void ima_rdwr_violation_check(struct file *file)
 {
-	struct dentry *dentry = file->f_path.dentry;
 	struct inode *inode = file_inode(file);
 	fmode_t mode = file->f_mode;
 	int must_measure;
@@ -111,8 +110,6 @@ out:
 		return;
 
 	pathname = ima_d_path(&file->f_path, &pathbuf);
-	if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX)
-		pathname = dentry->d_name.name;
 
 	if (send_tomtou)
 		ima_add_violation(file, pathname, "invalid_pcr", "ToMToU");
@@ -220,9 +217,7 @@ static int process_measurement(struct file *file, const char *filename,
 	if (rc != 0)
 		goto out_digsig;
 
-	pathname = !filename ? ima_d_path(&file->f_path, &pathbuf) : filename;
-	if (!pathname)
-		pathname = (const char *)file->f_dentry->d_name.name;
+	pathname = filename ?: ima_d_path(&file->f_path, &pathbuf);
 
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
-- 
1.8.3.2

[PATCH 4/8] evm: EVM does not use MD5

[Dmitry Kasatkin]

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index fea9749..5aa9103 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -2,7 +2,6 @@ config EVM
 	boolean "EVM support"
 	depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)
 	select CRYPTO_HMAC
-	select CRYPTO_MD5
 	select CRYPTO_SHA1
 	select ENCRYPTED_KEYS
 	default n
-- 
1.8.3.2

[PATCH 5/8] ima: skip memory allocation for empty files

[Dmitry Kasatkin]

Memory allocation is unnecessary for empty files.
This patch finalize the hash without memory allocation.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_crypto.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index d257e36..1bde8e6 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -87,16 +87,20 @@ static int ima_calc_file_hash_tfm(struct file *file,
 	if (rc != 0)
 		return rc;
 
-	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!rbuf) {
-		rc = -ENOMEM;
+	i_size = i_size_read(file_inode(file));
+
+	if (i_size == 0)
 		goto out;
-	}
+
+	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!rbuf)
+		return -ENOMEM;
+
 	if (!(file->f_mode & FMODE_READ)) {
 		file->f_mode |= FMODE_READ;
 		read = 1;
 	}
-	i_size = i_size_read(file_inode(file));
+
 	while (offset < i_size) {
 		int rbuf_len;
 
@@ -113,12 +117,12 @@ static int ima_calc_file_hash_tfm(struct file *file,
 		if (rc)
 			break;
 	}
-	kfree(rbuf);
-	if (!rc)
-		rc = crypto_shash_final(&desc.shash, hash->digest);
 	if (read)
 		file->f_mode &= ~FMODE_READ;
+	kfree(rbuf);
 out:
+	if (!rc)
+		rc = crypto_shash_final(&desc.shash, hash->digest);
 	return rc;
 }
 
-- 
1.8.3.2

[PATCH 6/8] evm: enable key retention service automatically

[Dmitry Kasatkin]

If keys are not enabled, EVM is not visible in the configuration menu.
It may be difficult to figure out what to do unless  you really know.

Other subsystems as NFS, CIFS select keys automatically.
This patch does the same.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index 5aa9103..d35b491 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -1,9 +1,10 @@
 config EVM
 	boolean "EVM support"
-	depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)
+	depends on SECURITY
+	select KEYS
+	select ENCRYPTED_KEYS
 	select CRYPTO_HMAC
 	select CRYPTO_SHA1
-	select ENCRYPTED_KEYS
 	default n
 	help
 	  EVM protects a file's security extended attributes against
-- 
1.8.3.2

[PATCH 7/8] evm: introduce EVM hmac attribute list

[Dmitry Kasatkin]

This patch replaces using of hmac version configuration parameter
with attribute list. It allows to build kernels which works with
previously labeled filesystems.

Currently supported attribute is 'fsuuid' which is equivalent of
former version 2.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig      | 19 ++++++++++---------
 security/integrity/evm/evm.h        |  4 +++-
 security/integrity/evm/evm_crypto.c |  2 +-
 security/integrity/evm/evm_main.c   | 21 ++++++++++++++++++++-
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index d35b491..2be51fa 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -12,15 +12,16 @@ config EVM
 
 	  If you are unsure how to answer this question, answer N.
 
-config EVM_HMAC_VERSION
-	int "EVM HMAC version"
-	depends on EVM
-	default 2
-	help
-	  This options adds EVM HMAC version support.
-	  1 - original version
-	  2 - add per filesystem unique identifier (UUID) (default)
+config EVM_HMAC_ATTRS
+	string "HMAC attributes"
+	default "fsuuid"
+ 	help
+	  This options allows to specify list of optional attributes included into HMAC
+	  calculation. It makes it possible easily upgrade to newer kernels.
+	 
+	  Default value is 'fsuuid', which is former version 2.
+	  if blank, it is equivalent of version 1
 
 	  WARNING: changing the HMAC calculation method or adding 
 	  additional info to the calculation, requires existing EVM
-	  labeled file systems to be relabeled.  
+	  labeled file systems to be relabeled.
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 37c88dd..c8fa0aa 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -24,11 +24,13 @@
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
-extern int evm_hmac_version;
+extern int evm_hmac_attrs;
 
 extern struct crypto_shash *hmac_tfm;
 extern struct crypto_shash *hash_tfm;
 
+#define EVM_HMAC_ATTR_FSUUID		0x0001
+
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index babd862..ab034e5 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
 	hmac_misc.mode = inode->i_mode;
 	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
-	if (evm_hmac_version > 1)
+	if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID)
 		crypto_shash_update(desc, inode->i_sb->s_uuid,
 				    sizeof(inode->i_sb->s_uuid));
 	crypto_shash_final(desc, digest);
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 996092f..9c05929 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -32,7 +32,7 @@ static char *integrity_status_msg[] = {
 };
 char *evm_hmac = "hmac(sha1)";
 char *evm_hash = "sha1";
-int evm_hmac_version = CONFIG_EVM_HMAC_VERSION;
+int evm_hmac_attrs;
 
 char *evm_config_xattrnames[] = {
 #ifdef CONFIG_SECURITY_SELINUX
@@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str)
 }
 __setup("evm=", evm_set_fixmode);
 
+static int __init evm_init_config(void)
+{
+	char *attrs = CONFIG_EVM_HMAC_ATTRS;
+	char *p;
+
+	while ((p = strsep(&attrs, ", \t"))) {
+		if (!strcmp(p, "fsuuid"))
+			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
+	}
+	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
+	return 0;
+}
+
 static int evm_find_protected_xattrs(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
@@ -432,6 +445,12 @@ static int __init init_evm(void)
 {
 	int error;
 
+	error = evm_init_config();
+	if (error < 0) {
+		pr_info("Error parsing config lists\n");
+		goto err;
+	}
+
 	error = evm_init_secfs();
 	if (error < 0) {
 		pr_info("Error registering secfs\n");
-- 
1.8.3.2

[PATCH 8/8] evm: introduce EVM hmac xattr list

[Dmitry Kasatkin]

EVM currently uses source hard coded list of xattrs which needs to be
included into the HMAC calculation. This is very unflexible.
Adding new attributes requires modifcation of the source code and
prevents building the kernel which works with previously labeled
filesystems.

Early versions of Smack used only one xattr security.SMACK64,
which is protected by EVM. Now Smack has a few more attributes and
they are not protected. Adding support to the source code makes it
impossible to use new kernel with previousely labeled filesystems.

This patch replaces hardcoded xattr array with dynamic list which is
initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
kernel with with support of older and newer EVM HMAC formats.

Possible future extension will be to read xattr list from the kernel
command line or from securityfs entry.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig      | 10 ++++++
 security/integrity/evm/evm.h        |  7 +++-
 security/integrity/evm/evm_crypto.c |  8 ++---
 security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
 4 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index 2be51fa..06237b8 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
 	  WARNING: changing the HMAC calculation method or adding 
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
+
+config EVM_HMAC_XATTRS
+	string "HMAC xattrs"
+	default "security.selinux security.SMACK64 security.ima security.capability"
+	help
+	  This options allows to specify list of extended attributes included into HMAC
+	  calculation. It makes it possible easily upgrade to newer kernels.
+
+	  Default value:
+	    security.selinux, security.SMACK64, security.ima, security.capability
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index c8fa0aa..4d1c51e 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
 
 #define EVM_HMAC_ATTR_FSUUID		0x0001
 
+struct evm_hmac_xattr {
+	struct list_head list;
+	char *xattr;
+};
+
 /* List of EVM protected security xattrs */
-extern char *evm_config_xattrnames[];
+extern struct list_head evm_hmac_xattrs;
 
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index ab034e5..7e5bfb7 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 {
 	struct inode *inode = dentry->d_inode;
 	struct shash_desc *desc;
-	char **xattrname;
+	struct evm_hmac_xattr *entry;
 	size_t xattr_size = 0;
 	char *xattr_value = NULL;
 	int error;
@@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		return PTR_ERR(desc);
 
 	error = -ENODATA;
-	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
 		if ((req_xattr_name && req_xattr_value)
-		    && !strcmp(*xattrname, req_xattr_name)) {
+		    && !strcmp(entry->xattr, req_xattr_name)) {
 			error = 0;
 			crypto_shash_update(desc, (const u8 *)req_xattr_value,
 					     req_xattr_value_len);
 			continue;
 		}
-		size = vfs_getxattr_alloc(dentry, *xattrname,
+		size = vfs_getxattr_alloc(dentry, entry->xattr,
 					  &xattr_value, xattr_size, GFP_NOFS);
 		if (size == -ENOMEM) {
 			error = -ENOMEM;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 9c05929..13e03ad 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
 char *evm_hash = "sha1";
 int evm_hmac_attrs;
 
-char *evm_config_xattrnames[] = {
-#ifdef CONFIG_SECURITY_SELINUX
-	XATTR_NAME_SELINUX,
-#endif
-#ifdef CONFIG_SECURITY_SMACK
-	XATTR_NAME_SMACK,
-#endif
-#ifdef CONFIG_IMA_APPRAISE
-	XATTR_NAME_IMA,
-#endif
-	XATTR_NAME_CAPS,
-	NULL
-};
+LIST_HEAD(evm_hmac_xattrs);
 
 static int evm_fixmode;
 static int __init evm_set_fixmode(char *str)
@@ -61,27 +49,53 @@ static int __init evm_init_config(void)
 {
 	char *attrs = CONFIG_EVM_HMAC_ATTRS;
 	char *p;
+	char *xattrs = CONFIG_EVM_HMAC_XATTRS;
+	struct evm_hmac_xattr *entry;
 
 	while ((p = strsep(&attrs, ", \t"))) {
 		if (!strcmp(p, "fsuuid"))
 			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
 	}
 	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
+	while ((p = strsep(&xattrs, ", \t"))) {
+#ifndef CONFIG_SECURITY_SELINUX
+		if (!strcmp(p, XATTR_NAME_SELINUX))
+			continue;
+#endif
+#ifndef CONFIG_SECURITY_SMACK
+		if (strstr(p, "SMACK64"))
+			continue;
+#endif
+#ifndef CONFIG_IMA_APPRAISE
+		if (!strcmp(p, XATTR_NAME_IMA))
+			continue;
+#endif
+		entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+		if (!entry)
+			return -ENOMEM;
+		INIT_LIST_HEAD(&entry->list);
+		entry->xattr = kstrdup(p, GFP_KERNEL);
+		if (!entry->xattr)
+			return -ENOMEM;
+		list_add_tail(&entry->list, &evm_hmac_xattrs);
+	}
+	list_for_each_entry(entry, &evm_hmac_xattrs, list)
+		pr_info("%s\n", entry->xattr);
 	return 0;
 }
 
 static int evm_find_protected_xattrs(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
-	char **xattr;
+	struct evm_hmac_xattr *entry;
 	int error;
 	int count = 0;
 
 	if (!inode->i_op || !inode->i_op->getxattr)
 		return -EOPNOTSUPP;
 
-	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
-		error = inode->i_op->getxattr(dentry, *xattr, NULL, 0);
+	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
+		error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0);
 		if (error < 0) {
 			if (error == -ENODATA)
 				continue;
@@ -183,19 +197,19 @@ out:
 
 static int evm_protected_xattr(const char *req_xattr_name)
 {
-	char **xattrname;
+	struct evm_hmac_xattr *entry;
 	int namelen;
 	int found = 0;
 
 	namelen = strlen(req_xattr_name);
-	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
-		if ((strlen(*xattrname) == namelen)
-		    && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
+	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
+		if ((strlen(entry->xattr) == namelen)
+		    && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) {
 			found = 1;
 			break;
 		}
 		if (strncmp(req_xattr_name,
-			    *xattrname + XATTR_SECURITY_PREFIX_LEN,
+			    entry->xattr + XATTR_SECURITY_PREFIX_LEN,
 			    strlen(req_xattr_name)) == 0) {
 			found = 1;
 			break;
@@ -462,19 +476,6 @@ err:
 	return error;
 }
 
-/*
- * evm_display_config - list the EVM protected security extended attributes
- */
-static int __init evm_display_config(void)
-{
-	char **xattrname;
-
-	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
-		pr_info("%s\n", *xattrname);
-	return 0;
-}
-
-pure_initcall(evm_display_config);
 late_initcall(init_evm);
 
 MODULE_DESCRIPTION("Extended Verification Module");
-- 
1.8.3.2

Re: [PATCH 2/8] integrity: fix checkpatch errors

[Dmitry Kasatkin]

This patch is on the top of Joe Perches patch.

- Dmitry

On 28/02/14 16:59, Dmitry Kasatkin wrote:
> Unfixed checkpatch errors make it difficult to see new errors..
> This patch fix them.
> Some lines with over 80 chars remained unchanged to improve
> code readability.
>
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  security/integrity/evm/evm.h          | 28 +++++++-------
>  security/integrity/evm/evm_crypto.c   |  4 +-
>  security/integrity/iint.c             |  2 +-
>  security/integrity/ima/ima_api.c      |  8 ++--
>  security/integrity/ima/ima_crypto.c   |  2 +-
>  security/integrity/ima/ima_fs.c       |  6 +--
>  security/integrity/ima/ima_main.c     |  4 +-
>  security/integrity/ima/ima_policy.c   | 69 +++++++++++++++++------------------
>  security/integrity/ima/ima_queue.c    |  4 +-
>  security/integrity/ima/ima_template.c | 14 +++----
>  security/integrity/integrity_audit.c  |  4 +-
>  11 files changed, 72 insertions(+), 73 deletions(-)
>
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index 30bd1ec..37c88dd 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm;
>  /* List of EVM protected security xattrs */
>  extern char *evm_config_xattrnames[];
>  
> -extern int evm_init_key(void);
> -extern int evm_update_evmxattr(struct dentry *dentry,
> -			       const char *req_xattr_name,
> -			       const char *req_xattr_value,
> -			       size_t req_xattr_value_len);
> -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> -			 const char *req_xattr_value,
> -			 size_t req_xattr_value_len, char *digest);
> -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> -			 const char *req_xattr_value,
> -			 size_t req_xattr_value_len, char *digest);
> -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> -			 char *hmac_val);
> -extern int evm_init_secfs(void);
> +int evm_init_key(void);
> +int evm_update_evmxattr(struct dentry *dentry,
> +			const char *req_xattr_name,
> +			const char *req_xattr_value,
> +			size_t req_xattr_value_len);
> +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> +		  const char *req_xattr_value,
> +		  size_t req_xattr_value_len, char *digest);
> +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> +		  const char *req_xattr_value,
> +		  size_t req_xattr_value_len, char *digest);
> +int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> +		  char *hmac_val);
> +int evm_init_secfs(void);
>  
>  #endif
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index 9bd329f..babd862 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>  		umode_t mode;
>  	} hmac_misc;
>  
> -	memset(&hmac_misc, 0, sizeof hmac_misc);
> +	memset(&hmac_misc, 0, sizeof(hmac_misc));
>  	hmac_misc.ino = inode->i_ino;
>  	hmac_misc.generation = inode->i_generation;
>  	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
>  	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
>  	hmac_misc.mode = inode->i_mode;
> -	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
> +	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
>  	if (evm_hmac_version > 1)
>  		crypto_shash_update(desc, inode->i_sb->s_uuid,
>  				    sizeof(inode->i_sb->s_uuid));
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index c49d3f1..a521edf 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -151,7 +151,7 @@ static void init_once(void *foo)
>  {
>  	struct integrity_iint_cache *iint = foo;
>  
> -	memset(iint, 0, sizeof *iint);
> +	memset(iint, 0, sizeof(*iint));
>  	iint->version = 0;
>  	iint->flags = 0UL;
>  	iint->ima_file_status = INTEGRITY_UNKNOWN;
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 393b9d4..c6b4a73 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -160,10 +160,10 @@ err_out:
>   * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK)
>   *
>   * The policy is defined in terms of keypairs:
> - * 		subj=, obj=, type=, func=, mask=, fsmagic=
> + *		subj=, obj=, type=, func=, mask=, fsmagic=
>   *	subj,obj, and type: are LSM specific.
> - * 	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> - * 	mask: contains the permission mask
> + *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> + *	mask: contains the permission mask
>   *	fsmagic: hex value
>   *
>   * Returns IMA_MEASURE, IMA_APPRAISE mask.
> @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>   *
>   * We only get here if the inode has not already been measured,
>   * but the measurement could already exist:
> - * 	- multiple copies of the same file on either the same or
> + *	- multiple copies of the same file on either the same or
>   *	  different filesystems.
>   *	- the inode was previously flushed as well as the iint info,
>   *	  containing the hashing info.
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 9999057..d257e36 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -10,7 +10,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * File: ima_crypto.c
> - * 	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
> + *	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 468a3ba..da92fcc 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v)
>  	 * PCR used is always the same (config option) in
>  	 * little-endian format
>  	 */
> -	ima_putc(m, &pcr, sizeof pcr);
> +	ima_putc(m, &pcr, sizeof(pcr));
>  
>  	/* 2nd: template digest */
>  	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
>  
>  	/* 3rd: template name size */
>  	namelen = strlen(e->template_desc->name);
> -	ima_putc(m, &namelen, sizeof namelen);
> +	ima_putc(m, &namelen, sizeof(namelen));
>  
>  	/* 4th:  template name */
>  	ima_putc(m, e->template_desc->name, namelen);
> @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1);
>  /*
>   * ima_open_policy: sequentialize access to the policy file
>   */
> -static int ima_open_policy(struct inode * inode, struct file * filp)
> +static int ima_open_policy(struct inode *inode, struct file *filp)
>  {
>  	/* No point in being allowed to open it if you aren't going to write */
>  	if (!(filp->f_flags & O_WRONLY))
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 149ee11..50413d0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup);
>   * ima_rdwr_violation_check
>   *
>   * Only invalidate the PCR for measured files:
> - * 	- Opening a file for write when already open for read,
> + *	- Opening a file for write when already open for read,
>   *	  results in a time of measure, time of use (ToMToU) error.
>   *	- Opening a file for read when already open for write,
> - * 	  could result in a file measurement error.
> + *	  could result in a file measurement error.
>   *
>   */
>  static void ima_rdwr_violation_check(struct file *file)
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 947cdbe..41021b4 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -7,7 +7,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * ima_policy.c
> - * 	- initialize default measure policy rules
> + *	- initialize default measure policy rules
>   *
>   */
>  #include <linux/module.h>
> @@ -21,8 +21,8 @@
>  #include "ima.h"
>  
>  /* flags definitions */
> -#define IMA_FUNC 	0x0001
> -#define IMA_MASK 	0x0002
> +#define IMA_FUNC	0x0001
> +#define IMA_MASK	0x0002
>  #define IMA_FSMAGIC	0x0004
>  #define IMA_UID		0x0008
>  #define IMA_FOWNER	0x0010
> @@ -69,35 +69,35 @@ struct ima_rule_entry {
>   * and running executables.
>   */
>  static struct ima_rule_entry default_rules[] = {
> -	{.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC,
> +	{.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
>  	 .flags = IMA_FUNC | IMA_MASK},
> -	{.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
> +	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
>  	 .flags = IMA_FUNC | IMA_MASK},
> -	{.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID,
> +	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID,
>  	 .flags = IMA_FUNC | IMA_MASK | IMA_UID},
> -	{.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC},
> +	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
>  };
>  
>  static struct ima_rule_entry default_appraise_rules[] = {
> -	{.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
> +	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
>  };
>  
>  static LIST_HEAD(ima_default_rules);
> @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str)
>  }
>  __setup("ima_appraise_tcb", default_appraise_policy_setup);
>  
> -/* 
> +/*
>   * Although the IMA policy does not change, the LSM policy can be
>   * reloaded, leaving the IMA LSM based rules referring to the old,
>   * stale LSM policy.
>   *
> - * Update the IMA LSM based rules to reflect the reloaded LSM policy. 
> + * Update the IMA LSM based rules to reflect the reloaded LSM policy.
>   * We assume the rules still exist; and BUG_ON() if they don't.
>   */
>  static void ima_lsm_update_rules(void)
> @@ -218,7 +218,7 @@ retry:
>  			retried = 1;
>  			ima_lsm_update_rules();
>  			goto retry;
> -		} 
> +		}
>  		if (!rc)
>  			return false;
>  	}
> @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
>  	if (!(rule->flags & IMA_FUNC))
>  		return IMA_FILE_APPRAISE;
>  
> -	switch(func) {
> +	switch (func) {
>  	case MMAP_CHECK:
>  		return IMA_MMAP_APPRAISE;
>  	case BPRM_CHECK:
> @@ -306,7 +306,7 @@ void __init ima_init_policy(void)
>  	measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0;
>  	appraise_entries = ima_use_appraise_tcb ?
>  			 ARRAY_SIZE(default_appraise_rules) : 0;
> -	
> +
>  	for (i = 0; i < measure_entries + appraise_entries; i++) {
>  		if (i < measure_entries)
>  			list_add_tail(&default_rules[i].list,
> @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
>  
> -			result = strict_strtoul(args[0].from, 16,
> -						&entry->fsmagic);
> +			result = kstrtoul(args[0].from, 16, &entry->fsmagic);
>  			if (!result)
>  				entry->flags |= IMA_FSMAGIC;
>  			break;
> @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
>  
> -			result = strict_strtoul(args[0].from, 10, &lnum);
> +			result = kstrtoul(args[0].from, 10, &lnum);
>  			if (!result) {
>  				entry->uid = make_kuid(current_user_ns(), (uid_t)lnum);
>  				if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum))
> @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
>  
> -			result = strict_strtoul(args[0].from, 10, &lnum);
> +			result = kstrtoul(args[0].from, 10, &lnum);
>  			if (!result) {
>  				entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum);
>  				if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum))
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 91128b4..552705d 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  
>  	mutex_lock(&ima_extend_list_mutex);
>  	if (!violation) {
> -		memcpy(digest, entry->digest, sizeof digest);
> +		memcpy(digest, entry->digest, sizeof(digest));
>  		if (ima_lookup_digest_entry(digest)) {
>  			audit_cause = "hash_exists";
>  			result = -EEXIST;
> @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  	}
>  
>  	if (violation)		/* invalidate pcr */
> -		memset(digest, 0xff, sizeof digest);
> +		memset(digest, 0xff, sizeof(digest));
>  
>  	tpmresult = ima_pcr_extend(digest);
>  	if (tpmresult != 0) {
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 9a4a0d1..a076a96 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -22,20 +22,20 @@
>  
>  static struct ima_template_desc defined_templates[] = {
>  	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> -	{.name = "ima-ng",.fmt = "d-ng|n-ng"},
> -	{.name = "ima-sig",.fmt = "d-ng|n-ng|sig"},
> +	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
> +	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>  };
>  
>  static struct ima_template_field supported_fields[] = {
> -	{.field_id = "d",.field_init = ima_eventdigest_init,
> +	{.field_id = "d", .field_init = ima_eventdigest_init,
>  	 .field_show = ima_show_template_digest},
> -	{.field_id = "n",.field_init = ima_eventname_init,
> +	{.field_id = "n", .field_init = ima_eventname_init,
>  	 .field_show = ima_show_template_string},
> -	{.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
> +	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
>  	 .field_show = ima_show_template_digest_ng},
> -	{.field_id = "n-ng",.field_init = ima_eventname_ng_init,
> +	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>  	 .field_show = ima_show_template_string},
> -	{.field_id = "sig",.field_init = ima_eventsig_init,
> +	{.field_id = "sig", .field_init = ima_eventsig_init,
>  	 .field_show = ima_show_template_sig},
>  };
>  
> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
> index 793d7be..aab9fa5 100644
> --- a/security/integrity/integrity_audit.c
> +++ b/security/integrity/integrity_audit.c
> @@ -7,7 +7,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * File: integrity_audit.c
> - * 	Audit calls for the integrity subsystem
> + *	Audit calls for the integrity subsystem
>   */
>  
>  #include <linux/fs.h>
> @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str)
>  {
>  	unsigned long audit;
>  
> -	if (!strict_strtoul(str, 0, &audit))
> +	if (!kstrtoul(str, 0, &audit))
>  		integrity_audit_info = audit ? 1 : 0;
>  	return 1;
>  }

Re: [PATCH 2/8] integrity: fix checkpatch errors

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> Unfixed checkpatch errors make it difficult to see new errors..
> This patch fix them.

A number of these errors are a result of inconsistencies between Lindent
and checkpatch.  This patch uses the checkpatch preferences.

> Some lines with over 80 chars remained unchanged to improve
> code readability.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

This patch doesn't apply cleanly to either linux-integrity/next-fixes or
to linux-security/next.

thanks,

Mimi

> ---
>  security/integrity/evm/evm.h          | 28 +++++++-------
>  security/integrity/evm/evm_crypto.c   |  4 +-
>  security/integrity/iint.c             |  2 +-
>  security/integrity/ima/ima_api.c      |  8 ++--
>  security/integrity/ima/ima_crypto.c   |  2 +-
>  security/integrity/ima/ima_fs.c       |  6 +--
>  security/integrity/ima/ima_main.c     |  4 +-
>  security/integrity/ima/ima_policy.c   | 69 +++++++++++++++++------------------
>  security/integrity/ima/ima_queue.c    |  4 +-
>  security/integrity/ima/ima_template.c | 14 +++----
>  security/integrity/integrity_audit.c  |  4 +-
>  11 files changed, 72 insertions(+), 73 deletions(-)
> 
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index 30bd1ec..37c88dd 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm;
>  /* List of EVM protected security xattrs */
>  extern char *evm_config_xattrnames[];
> 
> -extern int evm_init_key(void);
> -extern int evm_update_evmxattr(struct dentry *dentry,
> -			       const char *req_xattr_name,
> -			       const char *req_xattr_value,
> -			       size_t req_xattr_value_len);
> -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> -			 const char *req_xattr_value,
> -			 size_t req_xattr_value_len, char *digest);
> -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> -			 const char *req_xattr_value,
> -			 size_t req_xattr_value_len, char *digest);
> -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> -			 char *hmac_val);
> -extern int evm_init_secfs(void);
> +int evm_init_key(void);
> +int evm_update_evmxattr(struct dentry *dentry,
> +			const char *req_xattr_name,
> +			const char *req_xattr_value,
> +			size_t req_xattr_value_len);
> +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> +		  const char *req_xattr_value,
> +		  size_t req_xattr_value_len, char *digest);
> +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> +		  const char *req_xattr_value,
> +		  size_t req_xattr_value_len, char *digest);
> +int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> +		  char *hmac_val);
> +int evm_init_secfs(void);
> 
>  #endif
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index 9bd329f..babd862 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>  		umode_t mode;
>  	} hmac_misc;
> 
> -	memset(&hmac_misc, 0, sizeof hmac_misc);
> +	memset(&hmac_misc, 0, sizeof(hmac_misc));
>  	hmac_misc.ino = inode->i_ino;
>  	hmac_misc.generation = inode->i_generation;
>  	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
>  	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
>  	hmac_misc.mode = inode->i_mode;
> -	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
> +	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
>  	if (evm_hmac_version > 1)
>  		crypto_shash_update(desc, inode->i_sb->s_uuid,
>  				    sizeof(inode->i_sb->s_uuid));
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index c49d3f1..a521edf 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -151,7 +151,7 @@ static void init_once(void *foo)
>  {
>  	struct integrity_iint_cache *iint = foo;
> 
> -	memset(iint, 0, sizeof *iint);
> +	memset(iint, 0, sizeof(*iint));
>  	iint->version = 0;
>  	iint->flags = 0UL;
>  	iint->ima_file_status = INTEGRITY_UNKNOWN;
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 393b9d4..c6b4a73 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -160,10 +160,10 @@ err_out:
>   * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK)
>   *
>   * The policy is defined in terms of keypairs:
> - * 		subj=, obj=, type=, func=, mask=, fsmagic=
> + *		subj=, obj=, type=, func=, mask=, fsmagic=
>   *	subj,obj, and type: are LSM specific.
> - * 	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> - * 	mask: contains the permission mask
> + *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> + *	mask: contains the permission mask
>   *	fsmagic: hex value
>   *
>   * Returns IMA_MEASURE, IMA_APPRAISE mask.
> @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>   *
>   * We only get here if the inode has not already been measured,
>   * but the measurement could already exist:
> - * 	- multiple copies of the same file on either the same or
> + *	- multiple copies of the same file on either the same or
>   *	  different filesystems.
>   *	- the inode was previously flushed as well as the iint info,
>   *	  containing the hashing info.
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 9999057..d257e36 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -10,7 +10,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * File: ima_crypto.c
> - * 	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
> + *	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
>   */
> 
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 468a3ba..da92fcc 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v)
>  	 * PCR used is always the same (config option) in
>  	 * little-endian format
>  	 */
> -	ima_putc(m, &pcr, sizeof pcr);
> +	ima_putc(m, &pcr, sizeof(pcr));
> 
>  	/* 2nd: template digest */
>  	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
> 
>  	/* 3rd: template name size */
>  	namelen = strlen(e->template_desc->name);
> -	ima_putc(m, &namelen, sizeof namelen);
> +	ima_putc(m, &namelen, sizeof(namelen));
> 
>  	/* 4th:  template name */
>  	ima_putc(m, e->template_desc->name, namelen);
> @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1);
>  /*
>   * ima_open_policy: sequentialize access to the policy file
>   */
> -static int ima_open_policy(struct inode * inode, struct file * filp)
> +static int ima_open_policy(struct inode *inode, struct file *filp)
>  {
>  	/* No point in being allowed to open it if you aren't going to write */
>  	if (!(filp->f_flags & O_WRONLY))
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 149ee11..50413d0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup);
>   * ima_rdwr_violation_check
>   *
>   * Only invalidate the PCR for measured files:
> - * 	- Opening a file for write when already open for read,
> + *	- Opening a file for write when already open for read,
>   *	  results in a time of measure, time of use (ToMToU) error.
>   *	- Opening a file for read when already open for write,
> - * 	  could result in a file measurement error.
> + *	  could result in a file measurement error.
>   *
>   */
>  static void ima_rdwr_violation_check(struct file *file)
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 947cdbe..41021b4 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -7,7 +7,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * ima_policy.c
> - * 	- initialize default measure policy rules
> + *	- initialize default measure policy rules
>   *
>   */
>  #include <linux/module.h>
> @@ -21,8 +21,8 @@
>  #include "ima.h"
> 
>  /* flags definitions */
> -#define IMA_FUNC 	0x0001
> -#define IMA_MASK 	0x0002
> +#define IMA_FUNC	0x0001
> +#define IMA_MASK	0x0002
>  #define IMA_FSMAGIC	0x0004
>  #define IMA_UID		0x0008
>  #define IMA_FOWNER	0x0010
> @@ -69,35 +69,35 @@ struct ima_rule_entry {
>   * and running executables.
>   */
>  static struct ima_rule_entry default_rules[] = {
> -	{.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC,
> +	{.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
>  	 .flags = IMA_FUNC | IMA_MASK},
> -	{.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
> +	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
>  	 .flags = IMA_FUNC | IMA_MASK},
> -	{.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID,
> +	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID,
>  	 .flags = IMA_FUNC | IMA_MASK | IMA_UID},
> -	{.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC},
> +	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
>  };
> 
>  static struct ima_rule_entry default_appraise_rules[] = {
> -	{.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> -	{.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
> +	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
>  };
> 
>  static LIST_HEAD(ima_default_rules);
> @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str)
>  }
>  __setup("ima_appraise_tcb", default_appraise_policy_setup);
> 
> -/* 
> +/*
>   * Although the IMA policy does not change, the LSM policy can be
>   * reloaded, leaving the IMA LSM based rules referring to the old,
>   * stale LSM policy.
>   *
> - * Update the IMA LSM based rules to reflect the reloaded LSM policy. 
> + * Update the IMA LSM based rules to reflect the reloaded LSM policy.
>   * We assume the rules still exist; and BUG_ON() if they don't.
>   */
>  static void ima_lsm_update_rules(void)
> @@ -218,7 +218,7 @@ retry:
>  			retried = 1;
>  			ima_lsm_update_rules();
>  			goto retry;
> -		} 
> +		}
>  		if (!rc)
>  			return false;
>  	}
> @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
>  	if (!(rule->flags & IMA_FUNC))
>  		return IMA_FILE_APPRAISE;
> 
> -	switch(func) {
> +	switch (func) {
>  	case MMAP_CHECK:
>  		return IMA_MMAP_APPRAISE;
>  	case BPRM_CHECK:
> @@ -306,7 +306,7 @@ void __init ima_init_policy(void)
>  	measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0;
>  	appraise_entries = ima_use_appraise_tcb ?
>  			 ARRAY_SIZE(default_appraise_rules) : 0;
> -	
> +
>  	for (i = 0; i < measure_entries + appraise_entries; i++) {
>  		if (i < measure_entries)
>  			list_add_tail(&default_rules[i].list,
> @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
> 
> -			result = strict_strtoul(args[0].from, 16,
> -						&entry->fsmagic);
> +			result = kstrtoul(args[0].from, 16, &entry->fsmagic);
>  			if (!result)
>  				entry->flags |= IMA_FSMAGIC;
>  			break;
> @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
> 
> -			result = strict_strtoul(args[0].from, 10, &lnum);
> +			result = kstrtoul(args[0].from, 10, &lnum);
>  			if (!result) {
>  				entry->uid = make_kuid(current_user_ns(), (uid_t)lnum);
>  				if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum))
> @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				break;
>  			}
> 
> -			result = strict_strtoul(args[0].from, 10, &lnum);
> +			result = kstrtoul(args[0].from, 10, &lnum);
>  			if (!result) {
>  				entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum);
>  				if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum))
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 91128b4..552705d 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> 
>  	mutex_lock(&ima_extend_list_mutex);
>  	if (!violation) {
> -		memcpy(digest, entry->digest, sizeof digest);
> +		memcpy(digest, entry->digest, sizeof(digest));
>  		if (ima_lookup_digest_entry(digest)) {
>  			audit_cause = "hash_exists";
>  			result = -EEXIST;
> @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  	}
> 
>  	if (violation)		/* invalidate pcr */
> -		memset(digest, 0xff, sizeof digest);
> +		memset(digest, 0xff, sizeof(digest));
> 
>  	tpmresult = ima_pcr_extend(digest);
>  	if (tpmresult != 0) {
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 9a4a0d1..a076a96 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -22,20 +22,20 @@
> 
>  static struct ima_template_desc defined_templates[] = {
>  	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> -	{.name = "ima-ng",.fmt = "d-ng|n-ng"},
> -	{.name = "ima-sig",.fmt = "d-ng|n-ng|sig"},
> +	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
> +	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>  };
> 
>  static struct ima_template_field supported_fields[] = {
> -	{.field_id = "d",.field_init = ima_eventdigest_init,
> +	{.field_id = "d", .field_init = ima_eventdigest_init,
>  	 .field_show = ima_show_template_digest},
> -	{.field_id = "n",.field_init = ima_eventname_init,
> +	{.field_id = "n", .field_init = ima_eventname_init,
>  	 .field_show = ima_show_template_string},
> -	{.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
> +	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
>  	 .field_show = ima_show_template_digest_ng},
> -	{.field_id = "n-ng",.field_init = ima_eventname_ng_init,
> +	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>  	 .field_show = ima_show_template_string},
> -	{.field_id = "sig",.field_init = ima_eventsig_init,
> +	{.field_id = "sig", .field_init = ima_eventsig_init,
>  	 .field_show = ima_show_template_sig},
>  };
> 
> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
> index 793d7be..aab9fa5 100644
> --- a/security/integrity/integrity_audit.c
> +++ b/security/integrity/integrity_audit.c
> @@ -7,7 +7,7 @@
>   * the Free Software Foundation, version 2 of the License.
>   *
>   * File: integrity_audit.c
> - * 	Audit calls for the integrity subsystem
> + *	Audit calls for the integrity subsystem
>   */
> 
>  #include <linux/fs.h>
> @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str)
>  {
>  	unsigned long audit;
> 
> -	if (!strict_strtoul(str, 0, &audit))
> +	if (!kstrtoul(str, 0, &audit))
>  		integrity_audit_info = audit ? 1 : 0;
>  	return 1;
>  }

Re: [PATCH 1/8] ima: fix erronous removal of security.ima xattr

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> ima_inode_post_setattr() calls ima_must_appraise() to check if
> file needs to be appraised. If it is not then it removes security.ima
> xattr. With original policy matching code it might happen that even
> file needs to be appraised with FILE_CHECK hook, it might not be
> for POST_SETATTR hook. 'security.ima' might be erronously removed.
> 
> This patch treats POST_SETATTR as special wildcard function and will
> cause ima_must_appraise() to be true if any of the hooks rules matches.
> security.ima will not be removed if any of the hooks would require
> appraisal.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Thanks!

Mimi 
> ---
>  security/integrity/ima/ima_policy.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index d7c97a4..947cdbe 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -167,9 +167,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
>  	const struct cred *cred = current_cred();
>  	int i;
> 
> -	if ((rule->flags & IMA_FUNC) && rule->func != func)
> +	if ((rule->flags & IMA_FUNC) &&
> +		    (rule->func != func && func != POST_SETATTR))
>  		return false;
> -	if ((rule->flags & IMA_MASK) && rule->mask != mask)
> +	if ((rule->flags & IMA_MASK) &&
> +		    (rule->mask != mask && func != POST_SETATTR))
>  		return false;
>  	if ((rule->flags & IMA_FSMAGIC)
>  	    && rule->fsmagic != inode->i_sb->s_magic)

Re: [PATCH 3/8] ima: return d_name.name if d_path fails

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Nice refactoring.  Please add a patch description.

Mimi 
> ---
>  security/integrity/ima/ima_api.c  | 2 +-
>  security/integrity/ima/ima_main.c | 7 +------
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c6b4a73..ba9e4d7 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -332,5 +332,5 @@ const char *ima_d_path(struct path *path, char **pathbuf)
>  			pathname = NULL;
>  		}
>  	}
> -	return pathname;
> +	return pathname ?: (const char *)path->dentry->d_name.name;
>  }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 50413d0..52ac6cf 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -79,7 +79,6 @@ __setup("ima_hash=", hash_setup);
>   */
>  static void ima_rdwr_violation_check(struct file *file)
>  {
> -	struct dentry *dentry = file->f_path.dentry;
>  	struct inode *inode = file_inode(file);
>  	fmode_t mode = file->f_mode;
>  	int must_measure;
> @@ -111,8 +110,6 @@ out:
>  		return;
> 
>  	pathname = ima_d_path(&file->f_path, &pathbuf);
> -	if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX)
> -		pathname = dentry->d_name.name;
> 
>  	if (send_tomtou)
>  		ima_add_violation(file, pathname, "invalid_pcr", "ToMToU");
> @@ -220,9 +217,7 @@ static int process_measurement(struct file *file, const char *filename,
>  	if (rc != 0)
>  		goto out_digsig;
> 
> -	pathname = !filename ? ima_d_path(&file->f_path, &pathbuf) : filename;
> -	if (!pathname)
> -		pathname = (const char *)file->f_dentry->d_name.name;
> +	pathname = filename ?: ima_d_path(&file->f_path, &pathbuf);
> 
>  	if (action & IMA_MEASURE)
>  		ima_store_measurement(iint, file, pathname,

Re: [PATCH 2/8] integrity: fix checkpatch errors

[Dmitry Kasatkin]

On Mon, Mar 3, 2014 at 3:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> Unfixed checkpatch errors make it difficult to see new errors..
>> This patch fix them.
>
> A number of these errors are a result of inconsistencies between Lindent
> and checkpatch.  This patch uses the checkpatch preferences.
>
>> Some lines with over 80 chars remained unchanged to improve
>> code readability.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>
> This patch doesn't apply cleanly to either linux-integrity/next-fixes or
> to linux-security/next.
>
> thanks,
>

Hi,

Did you apply  Joe Perches's patch what you signed-off first?

- Dmitry

> Mimi
>
>> ---
>>  security/integrity/evm/evm.h          | 28 +++++++-------
>>  security/integrity/evm/evm_crypto.c   |  4 +-
>>  security/integrity/iint.c             |  2 +-
>>  security/integrity/ima/ima_api.c      |  8 ++--
>>  security/integrity/ima/ima_crypto.c   |  2 +-
>>  security/integrity/ima/ima_fs.c       |  6 +--
>>  security/integrity/ima/ima_main.c     |  4 +-
>>  security/integrity/ima/ima_policy.c   | 69 +++++++++++++++++------------------
>>  security/integrity/ima/ima_queue.c    |  4 +-
>>  security/integrity/ima/ima_template.c | 14 +++----
>>  security/integrity/integrity_audit.c  |  4 +-
>>  11 files changed, 72 insertions(+), 73 deletions(-)
>>
>> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> index 30bd1ec..37c88dd 100644
>> --- a/security/integrity/evm/evm.h
>> +++ b/security/integrity/evm/evm.h
>> @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm;
>>  /* List of EVM protected security xattrs */
>>  extern char *evm_config_xattrnames[];
>>
>> -extern int evm_init_key(void);
>> -extern int evm_update_evmxattr(struct dentry *dentry,
>> -                            const char *req_xattr_name,
>> -                            const char *req_xattr_value,
>> -                            size_t req_xattr_value_len);
>> -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
>> -                      const char *req_xattr_value,
>> -                      size_t req_xattr_value_len, char *digest);
>> -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
>> -                      const char *req_xattr_value,
>> -                      size_t req_xattr_value_len, char *digest);
>> -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
>> -                      char *hmac_val);
>> -extern int evm_init_secfs(void);
>> +int evm_init_key(void);
>> +int evm_update_evmxattr(struct dentry *dentry,
>> +                     const char *req_xattr_name,
>> +                     const char *req_xattr_value,
>> +                     size_t req_xattr_value_len);
>> +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
>> +               const char *req_xattr_value,
>> +               size_t req_xattr_value_len, char *digest);
>> +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
>> +               const char *req_xattr_value,
>> +               size_t req_xattr_value_len, char *digest);
>> +int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
>> +               char *hmac_val);
>> +int evm_init_secfs(void);
>>
>>  #endif
>> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> index 9bd329f..babd862 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>>               umode_t mode;
>>       } hmac_misc;
>>
>> -     memset(&hmac_misc, 0, sizeof hmac_misc);
>> +     memset(&hmac_misc, 0, sizeof(hmac_misc));
>>       hmac_misc.ino = inode->i_ino;
>>       hmac_misc.generation = inode->i_generation;
>>       hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
>>       hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
>>       hmac_misc.mode = inode->i_mode;
>> -     crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
>> +     crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
>>       if (evm_hmac_version > 1)
>>               crypto_shash_update(desc, inode->i_sb->s_uuid,
>>                                   sizeof(inode->i_sb->s_uuid));
>> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
>> index c49d3f1..a521edf 100644
>> --- a/security/integrity/iint.c
>> +++ b/security/integrity/iint.c
>> @@ -151,7 +151,7 @@ static void init_once(void *foo)
>>  {
>>       struct integrity_iint_cache *iint = foo;
>>
>> -     memset(iint, 0, sizeof *iint);
>> +     memset(iint, 0, sizeof(*iint));
>>       iint->version = 0;
>>       iint->flags = 0UL;
>>       iint->ima_file_status = INTEGRITY_UNKNOWN;
>> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
>> index 393b9d4..c6b4a73 100644
>> --- a/security/integrity/ima/ima_api.c
>> +++ b/security/integrity/ima/ima_api.c
>> @@ -160,10 +160,10 @@ err_out:
>>   * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK)
>>   *
>>   * The policy is defined in terms of keypairs:
>> - *           subj=, obj=, type=, func=, mask=, fsmagic=
>> + *           subj=, obj=, type=, func=, mask=, fsmagic=
>>   *   subj,obj, and type: are LSM specific.
>> - *   func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
>> - *   mask: contains the permission mask
>> + *   func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
>> + *   mask: contains the permission mask
>>   *   fsmagic: hex value
>>   *
>>   * Returns IMA_MEASURE, IMA_APPRAISE mask.
>> @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>>   *
>>   * We only get here if the inode has not already been measured,
>>   * but the measurement could already exist:
>> - *   - multiple copies of the same file on either the same or
>> + *   - multiple copies of the same file on either the same or
>>   *     different filesystems.
>>   *   - the inode was previously flushed as well as the iint info,
>>   *     containing the hashing info.
>> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
>> index 9999057..d257e36 100644
>> --- a/security/integrity/ima/ima_crypto.c
>> +++ b/security/integrity/ima/ima_crypto.c
>> @@ -10,7 +10,7 @@
>>   * the Free Software Foundation, version 2 of the License.
>>   *
>>   * File: ima_crypto.c
>> - *   Calculates md5/sha1 file hash, template hash, boot-aggreate hash
>> + *   Calculates md5/sha1 file hash, template hash, boot-aggreate hash
>>   */
>>
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index 468a3ba..da92fcc 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v)
>>        * PCR used is always the same (config option) in
>>        * little-endian format
>>        */
>> -     ima_putc(m, &pcr, sizeof pcr);
>> +     ima_putc(m, &pcr, sizeof(pcr));
>>
>>       /* 2nd: template digest */
>>       ima_putc(m, e->digest, TPM_DIGEST_SIZE);
>>
>>       /* 3rd: template name size */
>>       namelen = strlen(e->template_desc->name);
>> -     ima_putc(m, &namelen, sizeof namelen);
>> +     ima_putc(m, &namelen, sizeof(namelen));
>>
>>       /* 4th:  template name */
>>       ima_putc(m, e->template_desc->name, namelen);
>> @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1);
>>  /*
>>   * ima_open_policy: sequentialize access to the policy file
>>   */
>> -static int ima_open_policy(struct inode * inode, struct file * filp)
>> +static int ima_open_policy(struct inode *inode, struct file *filp)
>>  {
>>       /* No point in being allowed to open it if you aren't going to write */
>>       if (!(filp->f_flags & O_WRONLY))
>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>> index 149ee11..50413d0 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup);
>>   * ima_rdwr_violation_check
>>   *
>>   * Only invalidate the PCR for measured files:
>> - *   - Opening a file for write when already open for read,
>> + *   - Opening a file for write when already open for read,
>>   *     results in a time of measure, time of use (ToMToU) error.
>>   *   - Opening a file for read when already open for write,
>> - *     could result in a file measurement error.
>> + *     could result in a file measurement error.
>>   *
>>   */
>>  static void ima_rdwr_violation_check(struct file *file)
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 947cdbe..41021b4 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -7,7 +7,7 @@
>>   * the Free Software Foundation, version 2 of the License.
>>   *
>>   * ima_policy.c
>> - *   - initialize default measure policy rules
>> + *   - initialize default measure policy rules
>>   *
>>   */
>>  #include <linux/module.h>
>> @@ -21,8 +21,8 @@
>>  #include "ima.h"
>>
>>  /* flags definitions */
>> -#define IMA_FUNC     0x0001
>> -#define IMA_MASK     0x0002
>> +#define IMA_FUNC     0x0001
>> +#define IMA_MASK     0x0002
>>  #define IMA_FSMAGIC  0x0004
>>  #define IMA_UID              0x0008
>>  #define IMA_FOWNER   0x0010
>> @@ -69,35 +69,35 @@ struct ima_rule_entry {
>>   * and running executables.
>>   */
>>  static struct ima_rule_entry default_rules[] = {
>> -     {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC,
>> +     {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
>>        .flags = IMA_FUNC | IMA_MASK},
>> -     {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
>> +     {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
>>        .flags = IMA_FUNC | IMA_MASK},
>> -     {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID,
>> +     {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID,
>>        .flags = IMA_FUNC | IMA_MASK | IMA_UID},
>> -     {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC},
>> +     {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
>>  };
>>
>>  static struct ima_rule_entry default_appraise_rules[] = {
>> -     {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
>> -     {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
>> +     {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>> +     {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
>>  };
>>
>>  static LIST_HEAD(ima_default_rules);
>> @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str)
>>  }
>>  __setup("ima_appraise_tcb", default_appraise_policy_setup);
>>
>> -/*
>> +/*
>>   * Although the IMA policy does not change, the LSM policy can be
>>   * reloaded, leaving the IMA LSM based rules referring to the old,
>>   * stale LSM policy.
>>   *
>> - * Update the IMA LSM based rules to reflect the reloaded LSM policy.
>> + * Update the IMA LSM based rules to reflect the reloaded LSM policy.
>>   * We assume the rules still exist; and BUG_ON() if they don't.
>>   */
>>  static void ima_lsm_update_rules(void)
>> @@ -218,7 +218,7 @@ retry:
>>                       retried = 1;
>>                       ima_lsm_update_rules();
>>                       goto retry;
>> -             }
>> +             }
>>               if (!rc)
>>                       return false;
>>       }
>> @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
>>       if (!(rule->flags & IMA_FUNC))
>>               return IMA_FILE_APPRAISE;
>>
>> -     switch(func) {
>> +     switch (func) {
>>       case MMAP_CHECK:
>>               return IMA_MMAP_APPRAISE;
>>       case BPRM_CHECK:
>> @@ -306,7 +306,7 @@ void __init ima_init_policy(void)
>>       measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0;
>>       appraise_entries = ima_use_appraise_tcb ?
>>                        ARRAY_SIZE(default_appraise_rules) : 0;
>> -
>> +
>>       for (i = 0; i < measure_entries + appraise_entries; i++) {
>>               if (i < measure_entries)
>>                       list_add_tail(&default_rules[i].list,
>> @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>>                               break;
>>                       }
>>
>> -                     result = strict_strtoul(args[0].from, 16,
>> -                                             &entry->fsmagic);
>> +                     result = kstrtoul(args[0].from, 16, &entry->fsmagic);
>>                       if (!result)
>>                               entry->flags |= IMA_FSMAGIC;
>>                       break;
>> @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>>                               break;
>>                       }
>>
>> -                     result = strict_strtoul(args[0].from, 10, &lnum);
>> +                     result = kstrtoul(args[0].from, 10, &lnum);
>>                       if (!result) {
>>                               entry->uid = make_kuid(current_user_ns(), (uid_t)lnum);
>>                               if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum))
>> @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>>                               break;
>>                       }
>>
>> -                     result = strict_strtoul(args[0].from, 10, &lnum);
>> +                     result = kstrtoul(args[0].from, 10, &lnum);
>>                       if (!result) {
>>                               entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum);
>>                               if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum))
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index 91128b4..552705d 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>
>>       mutex_lock(&ima_extend_list_mutex);
>>       if (!violation) {
>> -             memcpy(digest, entry->digest, sizeof digest);
>> +             memcpy(digest, entry->digest, sizeof(digest));
>>               if (ima_lookup_digest_entry(digest)) {
>>                       audit_cause = "hash_exists";
>>                       result = -EEXIST;
>> @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>       }
>>
>>       if (violation)          /* invalidate pcr */
>> -             memset(digest, 0xff, sizeof digest);
>> +             memset(digest, 0xff, sizeof(digest));
>>
>>       tpmresult = ima_pcr_extend(digest);
>>       if (tpmresult != 0) {
>> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
>> index 9a4a0d1..a076a96 100644
>> --- a/security/integrity/ima/ima_template.c
>> +++ b/security/integrity/ima/ima_template.c
>> @@ -22,20 +22,20 @@
>>
>>  static struct ima_template_desc defined_templates[] = {
>>       {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
>> -     {.name = "ima-ng",.fmt = "d-ng|n-ng"},
>> -     {.name = "ima-sig",.fmt = "d-ng|n-ng|sig"},
>> +     {.name = "ima-ng", .fmt = "d-ng|n-ng"},
>> +     {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>>  };
>>
>>  static struct ima_template_field supported_fields[] = {
>> -     {.field_id = "d",.field_init = ima_eventdigest_init,
>> +     {.field_id = "d", .field_init = ima_eventdigest_init,
>>        .field_show = ima_show_template_digest},
>> -     {.field_id = "n",.field_init = ima_eventname_init,
>> +     {.field_id = "n", .field_init = ima_eventname_init,
>>        .field_show = ima_show_template_string},
>> -     {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
>> +     {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
>>        .field_show = ima_show_template_digest_ng},
>> -     {.field_id = "n-ng",.field_init = ima_eventname_ng_init,
>> +     {.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>>        .field_show = ima_show_template_string},
>> -     {.field_id = "sig",.field_init = ima_eventsig_init,
>> +     {.field_id = "sig", .field_init = ima_eventsig_init,
>>        .field_show = ima_show_template_sig},
>>  };
>>
>> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
>> index 793d7be..aab9fa5 100644
>> --- a/security/integrity/integrity_audit.c
>> +++ b/security/integrity/integrity_audit.c
>> @@ -7,7 +7,7 @@
>>   * the Free Software Foundation, version 2 of the License.
>>   *
>>   * File: integrity_audit.c
>> - *   Audit calls for the integrity subsystem
>> + *   Audit calls for the integrity subsystem
>>   */
>>
>>  #include <linux/fs.h>
>> @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str)
>>  {
>>       unsigned long audit;
>>
>> -     if (!strict_strtoul(str, 0, &audit))
>> +     if (!kstrtoul(str, 0, &audit))
>>               integrity_audit_info = audit ? 1 : 0;
>>       return 1;
>>  }
>
>



-- 
Thanks,
Dmitry

Re: [PATCH 2/8] integrity: fix checkpatch errors

[Mimi Zohar]

On Mon, 2014-03-03 at 16:01 +0200, Dmitry Kasatkin wrote: 
> On Mon, Mar 3, 2014 at 3:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
> >> Unfixed checkpatch errors make it difficult to see new errors..
> >> This patch fix them.
> >
> > A number of these errors are a result of inconsistencies between Lindent
> > and checkpatch.  This patch uses the checkpatch preferences.
> >
> >> Some lines with over 80 chars remained unchanged to improve
> >> code readability.
> >>
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >
> > This patch doesn't apply cleanly to either linux-integrity/next-fixes or
> > to linux-security/next.
> >
> > thanks,
> >
> 
> Hi,
> 
> Did you apply  Joe Perches's patch what you signed-off first?

Thanks for the reminder. I hadn't applied Joe's patch, but that didn't
resolve it.

Mimi

Re: [PATCH 6/8] evm: enable key retention service automatically

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> If keys are not enabled, EVM is not visible in the configuration menu.
> It may be difficult to figure out what to do unless  you really know.
> 
> Other subsystems as NFS, CIFS select keys automatically.
> This patch does the same.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  security/integrity/evm/Kconfig | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> index 5aa9103..d35b491 100644
> --- a/security/integrity/evm/Kconfig
> +++ b/security/integrity/evm/Kconfig
> @@ -1,9 +1,10 @@
>  config EVM
>  	boolean "EVM support"
> -	depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)

Including KEYS is fine, but the trusted-keys dependency is still
required.  If trusted-keys is enabled, then the TPM and trusted-keys
must be builtin.

Mimi

> +	depends on SECURITY
> +	select KEYS
> +	select ENCRYPTED_KEYS
>  	select CRYPTO_HMAC
>  	select CRYPTO_SHA1
> -	select ENCRYPTED_KEYS
>  	default n
>  	help
>  	  EVM protects a file's security extended attributes against

Re: [PATCH 4/8] evm: EVM does not use MD5

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Missing patch description ...

Mimi 
> ---
>  security/integrity/evm/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> index fea9749..5aa9103 100644
> --- a/security/integrity/evm/Kconfig
> +++ b/security/integrity/evm/Kconfig
> @@ -2,7 +2,6 @@ config EVM
>  	boolean "EVM support"
>  	depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)
>  	select CRYPTO_HMAC
> -	select CRYPTO_MD5
>  	select CRYPTO_SHA1
>  	select ENCRYPTED_KEYS
>  	default n

Re: [PATCH 2/8] integrity: fix checkpatch errors

[Mimi Zohar]

On Mon, 2014-03-03 at 08:41 -0500, Mimi Zohar wrote: 
> On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> > Unfixed checkpatch errors make it difficult to see new errors..
> > This patch fix them.

Between checkpatch changes (eg. sizeof) and inconsistencies between
Lindent and checkpatch, unfixed checkpatch errors make it difficult to
see new errors.  This patch fixes them.

checkpatch doesn't complain about 'evm.h'.  Please include the reason
for the change in the patch description.

> > Some lines with over 80 chars remained unchanged to improve
> > code readability.
> > 
> > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> 
> This patch doesn't apply cleanly to either linux-integrity/next-fixes or
> to linux-security/next.

Thanks for rebasing against linux-integrity/next-fixes.

Mimi

> 
> > ---
> >  security/integrity/evm/evm.h          | 28 +++++++-------
> >  security/integrity/evm/evm_crypto.c   |  4 +-
> >  security/integrity/iint.c             |  2 +-
> >  security/integrity/ima/ima_api.c      |  8 ++--
> >  security/integrity/ima/ima_crypto.c   |  2 +-
> >  security/integrity/ima/ima_fs.c       |  6 +--
> >  security/integrity/ima/ima_main.c     |  4 +-
> >  security/integrity/ima/ima_policy.c   | 69 +++++++++++++++++------------------
> >  security/integrity/ima/ima_queue.c    |  4 +-
> >  security/integrity/ima/ima_template.c | 14 +++----
> >  security/integrity/integrity_audit.c  |  4 +-
> >  11 files changed, 72 insertions(+), 73 deletions(-)
> > 
> > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> > index 30bd1ec..37c88dd 100644
> > --- a/security/integrity/evm/evm.h
> > +++ b/security/integrity/evm/evm.h
> > @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm;
> >  /* List of EVM protected security xattrs */
> >  extern char *evm_config_xattrnames[];
> > 
> > -extern int evm_init_key(void);
> > -extern int evm_update_evmxattr(struct dentry *dentry,
> > -			       const char *req_xattr_name,
> > -			       const char *req_xattr_value,
> > -			       size_t req_xattr_value_len);
> > -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> > -			 const char *req_xattr_value,
> > -			 size_t req_xattr_value_len, char *digest);
> > -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> > -			 const char *req_xattr_value,
> > -			 size_t req_xattr_value_len, char *digest);
> > -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> > -			 char *hmac_val);
> > -extern int evm_init_secfs(void);
> > +int evm_init_key(void);
> > +int evm_update_evmxattr(struct dentry *dentry,
> > +			const char *req_xattr_name,
> > +			const char *req_xattr_value,
> > +			size_t req_xattr_value_len);
> > +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> > +		  const char *req_xattr_value,
> > +		  size_t req_xattr_value_len, char *digest);
> > +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> > +		  const char *req_xattr_value,
> > +		  size_t req_xattr_value_len, char *digest);
> > +int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
> > +		  char *hmac_val);
> > +int evm_init_secfs(void);
> > 
> >  #endif
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index 9bd329f..babd862 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> >  		umode_t mode;
> >  	} hmac_misc;
> > 
> > -	memset(&hmac_misc, 0, sizeof hmac_misc);
> > +	memset(&hmac_misc, 0, sizeof(hmac_misc));
> >  	hmac_misc.ino = inode->i_ino;
> >  	hmac_misc.generation = inode->i_generation;
> >  	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
> >  	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
> >  	hmac_misc.mode = inode->i_mode;
> > -	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
> > +	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> >  	if (evm_hmac_version > 1)
> >  		crypto_shash_update(desc, inode->i_sb->s_uuid,
> >  				    sizeof(inode->i_sb->s_uuid));
> > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > index c49d3f1..a521edf 100644
> > --- a/security/integrity/iint.c
> > +++ b/security/integrity/iint.c
> > @@ -151,7 +151,7 @@ static void init_once(void *foo)
> >  {
> >  	struct integrity_iint_cache *iint = foo;
> > 
> > -	memset(iint, 0, sizeof *iint);
> > +	memset(iint, 0, sizeof(*iint));
> >  	iint->version = 0;
> >  	iint->flags = 0UL;
> >  	iint->ima_file_status = INTEGRITY_UNKNOWN;
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index 393b9d4..c6b4a73 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -160,10 +160,10 @@ err_out:
> >   * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK)
> >   *
> >   * The policy is defined in terms of keypairs:
> > - * 		subj=, obj=, type=, func=, mask=, fsmagic=
> > + *		subj=, obj=, type=, func=, mask=, fsmagic=
> >   *	subj,obj, and type: are LSM specific.
> > - * 	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> > - * 	mask: contains the permission mask
> > + *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> > + *	mask: contains the permission mask
> >   *	fsmagic: hex value
> >   *
> >   * Returns IMA_MEASURE, IMA_APPRAISE mask.
> > @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
> >   *
> >   * We only get here if the inode has not already been measured,
> >   * but the measurement could already exist:
> > - * 	- multiple copies of the same file on either the same or
> > + *	- multiple copies of the same file on either the same or
> >   *	  different filesystems.
> >   *	- the inode was previously flushed as well as the iint info,
> >   *	  containing the hashing info.
> > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> > index 9999057..d257e36 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -10,7 +10,7 @@
> >   * the Free Software Foundation, version 2 of the License.
> >   *
> >   * File: ima_crypto.c
> > - * 	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
> > + *	Calculates md5/sha1 file hash, template hash, boot-aggreate hash
> >   */
> > 
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 468a3ba..da92fcc 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v)
> >  	 * PCR used is always the same (config option) in
> >  	 * little-endian format
> >  	 */
> > -	ima_putc(m, &pcr, sizeof pcr);
> > +	ima_putc(m, &pcr, sizeof(pcr));
> > 
> >  	/* 2nd: template digest */
> >  	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
> > 
> >  	/* 3rd: template name size */
> >  	namelen = strlen(e->template_desc->name);
> > -	ima_putc(m, &namelen, sizeof namelen);
> > +	ima_putc(m, &namelen, sizeof(namelen));
> > 
> >  	/* 4th:  template name */
> >  	ima_putc(m, e->template_desc->name, namelen);
> > @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1);
> >  /*
> >   * ima_open_policy: sequentialize access to the policy file
> >   */
> > -static int ima_open_policy(struct inode * inode, struct file * filp)
> > +static int ima_open_policy(struct inode *inode, struct file *filp)
> >  {
> >  	/* No point in being allowed to open it if you aren't going to write */
> >  	if (!(filp->f_flags & O_WRONLY))
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 149ee11..50413d0 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup);
> >   * ima_rdwr_violation_check
> >   *
> >   * Only invalidate the PCR for measured files:
> > - * 	- Opening a file for write when already open for read,
> > + *	- Opening a file for write when already open for read,
> >   *	  results in a time of measure, time of use (ToMToU) error.
> >   *	- Opening a file for read when already open for write,
> > - * 	  could result in a file measurement error.
> > + *	  could result in a file measurement error.
> >   *
> >   */
> >  static void ima_rdwr_violation_check(struct file *file)
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index 947cdbe..41021b4 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -7,7 +7,7 @@
> >   * the Free Software Foundation, version 2 of the License.
> >   *
> >   * ima_policy.c
> > - * 	- initialize default measure policy rules
> > + *	- initialize default measure policy rules
> >   *
> >   */
> >  #include <linux/module.h>
> > @@ -21,8 +21,8 @@
> >  #include "ima.h"
> > 
> >  /* flags definitions */
> > -#define IMA_FUNC 	0x0001
> > -#define IMA_MASK 	0x0002
> > +#define IMA_FUNC	0x0001
> > +#define IMA_MASK	0x0002
> >  #define IMA_FSMAGIC	0x0004
> >  #define IMA_UID		0x0008
> >  #define IMA_FOWNER	0x0010
> > @@ -69,35 +69,35 @@ struct ima_rule_entry {
> >   * and running executables.
> >   */
> >  static struct ima_rule_entry default_rules[] = {
> > -	{.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC,
> > +	{.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
> >  	 .flags = IMA_FUNC | IMA_MASK},
> > -	{.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
> > +	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
> >  	 .flags = IMA_FUNC | IMA_MASK},
> > -	{.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID,
> > +	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID,
> >  	 .flags = IMA_FUNC | IMA_MASK | IMA_UID},
> > -	{.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC},
> > +	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
> >  };
> > 
> >  static struct ima_rule_entry default_appraise_rules[] = {
> > -	{.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
> > -	{.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
> > +	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> > +	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
> >  };
> > 
> >  static LIST_HEAD(ima_default_rules);
> > @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str)
> >  }
> >  __setup("ima_appraise_tcb", default_appraise_policy_setup);
> > 
> > -/* 
> > +/*
> >   * Although the IMA policy does not change, the LSM policy can be
> >   * reloaded, leaving the IMA LSM based rules referring to the old,
> >   * stale LSM policy.
> >   *
> > - * Update the IMA LSM based rules to reflect the reloaded LSM policy. 
> > + * Update the IMA LSM based rules to reflect the reloaded LSM policy.
> >   * We assume the rules still exist; and BUG_ON() if they don't.
> >   */
> >  static void ima_lsm_update_rules(void)
> > @@ -218,7 +218,7 @@ retry:
> >  			retried = 1;
> >  			ima_lsm_update_rules();
> >  			goto retry;
> > -		} 
> > +		}
> >  		if (!rc)
> >  			return false;
> >  	}
> > @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
> >  	if (!(rule->flags & IMA_FUNC))
> >  		return IMA_FILE_APPRAISE;
> > 
> > -	switch(func) {
> > +	switch (func) {
> >  	case MMAP_CHECK:
> >  		return IMA_MMAP_APPRAISE;
> >  	case BPRM_CHECK:
> > @@ -306,7 +306,7 @@ void __init ima_init_policy(void)
> >  	measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0;
> >  	appraise_entries = ima_use_appraise_tcb ?
> >  			 ARRAY_SIZE(default_appraise_rules) : 0;
> > -	
> > +
> >  	for (i = 0; i < measure_entries + appraise_entries; i++) {
> >  		if (i < measure_entries)
> >  			list_add_tail(&default_rules[i].list,
> > @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> >  				break;
> >  			}
> > 
> > -			result = strict_strtoul(args[0].from, 16,
> > -						&entry->fsmagic);
> > +			result = kstrtoul(args[0].from, 16, &entry->fsmagic);
> >  			if (!result)
> >  				entry->flags |= IMA_FSMAGIC;
> >  			break;
> > @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> >  				break;
> >  			}
> > 
> > -			result = strict_strtoul(args[0].from, 10, &lnum);
> > +			result = kstrtoul(args[0].from, 10, &lnum);
> >  			if (!result) {
> >  				entry->uid = make_kuid(current_user_ns(), (uid_t)lnum);
> >  				if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum))
> > @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> >  				break;
> >  			}
> > 
> > -			result = strict_strtoul(args[0].from, 10, &lnum);
> > +			result = kstrtoul(args[0].from, 10, &lnum);
> >  			if (!result) {
> >  				entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum);
> >  				if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum))
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index 91128b4..552705d 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> > 
> >  	mutex_lock(&ima_extend_list_mutex);
> >  	if (!violation) {
> > -		memcpy(digest, entry->digest, sizeof digest);
> > +		memcpy(digest, entry->digest, sizeof(digest));
> >  		if (ima_lookup_digest_entry(digest)) {
> >  			audit_cause = "hash_exists";
> >  			result = -EEXIST;
> > @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> >  	}
> > 
> >  	if (violation)		/* invalidate pcr */
> > -		memset(digest, 0xff, sizeof digest);
> > +		memset(digest, 0xff, sizeof(digest));
> > 
> >  	tpmresult = ima_pcr_extend(digest);
> >  	if (tpmresult != 0) {
> > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> > index 9a4a0d1..a076a96 100644
> > --- a/security/integrity/ima/ima_template.c
> > +++ b/security/integrity/ima/ima_template.c
> > @@ -22,20 +22,20 @@
> > 
> >  static struct ima_template_desc defined_templates[] = {
> >  	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
> > -	{.name = "ima-ng",.fmt = "d-ng|n-ng"},
> > -	{.name = "ima-sig",.fmt = "d-ng|n-ng|sig"},
> > +	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
> > +	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> >  };
> > 
> >  static struct ima_template_field supported_fields[] = {
> > -	{.field_id = "d",.field_init = ima_eventdigest_init,
> > +	{.field_id = "d", .field_init = ima_eventdigest_init,
> >  	 .field_show = ima_show_template_digest},
> > -	{.field_id = "n",.field_init = ima_eventname_init,
> > +	{.field_id = "n", .field_init = ima_eventname_init,
> >  	 .field_show = ima_show_template_string},
> > -	{.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
> > +	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
> >  	 .field_show = ima_show_template_digest_ng},
> > -	{.field_id = "n-ng",.field_init = ima_eventname_ng_init,
> > +	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
> >  	 .field_show = ima_show_template_string},
> > -	{.field_id = "sig",.field_init = ima_eventsig_init,
> > +	{.field_id = "sig", .field_init = ima_eventsig_init,
> >  	 .field_show = ima_show_template_sig},
> >  };
> > 
> > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
> > index 793d7be..aab9fa5 100644
> > --- a/security/integrity/integrity_audit.c
> > +++ b/security/integrity/integrity_audit.c
> > @@ -7,7 +7,7 @@
> >   * the Free Software Foundation, version 2 of the License.
> >   *
> >   * File: integrity_audit.c
> > - * 	Audit calls for the integrity subsystem
> > + *	Audit calls for the integrity subsystem
> >   */
> > 
> >  #include <linux/fs.h>
> > @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str)
> >  {
> >  	unsigned long audit;
> > 
> > -	if (!strict_strtoul(str, 0, &audit))
> > +	if (!kstrtoul(str, 0, &audit))
> >  		integrity_audit_info = audit ? 1 : 0;
> >  	return 1;
> >  }
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/8] ima: skip memory allocation for empty files

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> Memory allocation is unnecessary for empty files.
> This patch finalize the hash without memory allocation.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Thanks.

Mimi 
> ---
>  security/integrity/ima/ima_crypto.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index d257e36..1bde8e6 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -87,16 +87,20 @@ static int ima_calc_file_hash_tfm(struct file *file,
>  	if (rc != 0)
>  		return rc;
> 
> -	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> -	if (!rbuf) {
> -		rc = -ENOMEM;
> +	i_size = i_size_read(file_inode(file));
> +
> +	if (i_size == 0)
>  		goto out;
> -	}
> +
> +	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!rbuf)
> +		return -ENOMEM;
> +
>  	if (!(file->f_mode & FMODE_READ)) {
>  		file->f_mode |= FMODE_READ;
>  		read = 1;
>  	}
> -	i_size = i_size_read(file_inode(file));
> +
>  	while (offset < i_size) {
>  		int rbuf_len;
> 
> @@ -113,12 +117,12 @@ static int ima_calc_file_hash_tfm(struct file *file,
>  		if (rc)
>  			break;
>  	}
> -	kfree(rbuf);
> -	if (!rc)
> -		rc = crypto_shash_final(&desc.shash, hash->digest);
>  	if (read)
>  		file->f_mode &= ~FMODE_READ;
> +	kfree(rbuf);
>  out:
> +	if (!rc)
> +		rc = crypto_shash_final(&desc.shash, hash->digest);
>  	return rc;
>  }
> 

Re: [PATCH 7/8] evm: introduce EVM hmac attribute list

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> This patch replaces using of hmac version configuration parameter
> with attribute list. It allows to build kernels which works with
> previously labeled filesystems.
> 
> Currently supported attribute is 'fsuuid' which is equivalent of
> former version 2.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Please include the new boot command line option in
Documentation/kernel-parameters.txt.

Mimi

> ---
>  security/integrity/evm/Kconfig      | 19 ++++++++++---------
>  security/integrity/evm/evm.h        |  4 +++-
>  security/integrity/evm/evm_crypto.c |  2 +-
>  security/integrity/evm/evm_main.c   | 21 ++++++++++++++++++++-
>  4 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> index d35b491..2be51fa 100644
> --- a/security/integrity/evm/Kconfig
> +++ b/security/integrity/evm/Kconfig
> @@ -12,15 +12,16 @@ config EVM
> 
>  	  If you are unsure how to answer this question, answer N.
> 
> -config EVM_HMAC_VERSION
> -	int "EVM HMAC version"
> -	depends on EVM
> -	default 2
> -	help
> -	  This options adds EVM HMAC version support.
> -	  1 - original version
> -	  2 - add per filesystem unique identifier (UUID) (default)
> +config EVM_HMAC_ATTRS
> +	string "HMAC attributes"
> +	default "fsuuid"
> + 	help
> +	  This options allows to specify list of optional attributes included into HMAC
> +	  calculation. It makes it possible easily upgrade to newer kernels.
> +	 
> +	  Default value is 'fsuuid', which is former version 2.
> +	  if blank, it is equivalent of version 1
> 
>  	  WARNING: changing the HMAC calculation method or adding 
>  	  additional info to the calculation, requires existing EVM
> -	  labeled file systems to be relabeled.  
> +	  labeled file systems to be relabeled.
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index 37c88dd..c8fa0aa 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -24,11 +24,13 @@
>  extern int evm_initialized;
>  extern char *evm_hmac;
>  extern char *evm_hash;
> -extern int evm_hmac_version;
> +extern int evm_hmac_attrs;
> 
>  extern struct crypto_shash *hmac_tfm;
>  extern struct crypto_shash *hash_tfm;
> 
> +#define EVM_HMAC_ATTR_FSUUID		0x0001
> +
>  /* List of EVM protected security xattrs */
>  extern char *evm_config_xattrnames[];
> 
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index babd862..ab034e5 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>  	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
>  	hmac_misc.mode = inode->i_mode;
>  	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> -	if (evm_hmac_version > 1)
> +	if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID)
>  		crypto_shash_update(desc, inode->i_sb->s_uuid,
>  				    sizeof(inode->i_sb->s_uuid));
>  	crypto_shash_final(desc, digest);
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 996092f..9c05929 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -32,7 +32,7 @@ static char *integrity_status_msg[] = {
>  };
>  char *evm_hmac = "hmac(sha1)";
>  char *evm_hash = "sha1";
> -int evm_hmac_version = CONFIG_EVM_HMAC_VERSION;
> +int evm_hmac_attrs;
> 
>  char *evm_config_xattrnames[] = {
>  #ifdef CONFIG_SECURITY_SELINUX
> @@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str)
>  }
>  __setup("evm=", evm_set_fixmode);
> 
> +static int __init evm_init_config(void)
> +{
> +	char *attrs = CONFIG_EVM_HMAC_ATTRS;
> +	char *p;
> +
> +	while ((p = strsep(&attrs, ", \t"))) {
> +		if (!strcmp(p, "fsuuid"))
> +			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
> +	}
> +	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
> +	return 0;
> +}
> +
>  static int evm_find_protected_xattrs(struct dentry *dentry)
>  {
>  	struct inode *inode = dentry->d_inode;
> @@ -432,6 +445,12 @@ static int __init init_evm(void)
>  {
>  	int error;
> 
> +	error = evm_init_config();
> +	if (error < 0) {
> +		pr_info("Error parsing config lists\n");
> +		goto err;
> +	}
> +
>  	error = evm_init_secfs();
>  	if (error < 0) {
>  		pr_info("Error registering secfs\n");

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Mimi Zohar]

On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> EVM currently uses source hard coded list of xattrs which needs to be
> included into the HMAC calculation. This is very unflexible.
> Adding new attributes requires modifcation of the source code and
> prevents building the kernel which works with previously labeled
> filesystems.
> 
> Early versions of Smack used only one xattr security.SMACK64,
> which is protected by EVM. Now Smack has a few more attributes and
> they are not protected. Adding support to the source code makes it
> impossible to use new kernel with previousely labeled filesystems.

I think this patch will break any existing filesystems labeled with
'security.smack64'.  Details inline.

> This patch replaces hardcoded xattr array with dynamic list which is
> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
> kernel with with support of older and newer EVM HMAC formats.
> 
> Possible future extension will be to read xattr list from the kernel
> command line or from securityfs entry.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  security/integrity/evm/Kconfig      | 10 ++++++
>  security/integrity/evm/evm.h        |  7 +++-
>  security/integrity/evm/evm_crypto.c |  8 ++---
>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
>  4 files changed, 55 insertions(+), 39 deletions(-)
> 
> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> index 2be51fa..06237b8 100644
> --- a/security/integrity/evm/Kconfig
> +++ b/security/integrity/evm/Kconfig
> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
>  	  WARNING: changing the HMAC calculation method or adding 
>  	  additional info to the calculation, requires existing EVM
>  	  labeled file systems to be relabeled.
> +
> +config EVM_HMAC_XATTRS
> +	string "HMAC xattrs"
> +	default "security.selinux security.SMACK64 security.ima security.capability"
> +	help
> +	  This options allows to specify list of extended attributes included into HMAC
> +	  calculation. It makes it possible easily upgrade to newer kernels.
> +
> +	  Default value:
> +	    security.selinux, security.SMACK64, security.ima, security.capability
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index c8fa0aa..4d1c51e 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
> 
>  #define EVM_HMAC_ATTR_FSUUID		0x0001
> 
> +struct evm_hmac_xattr {
> +	struct list_head list;
> +	char *xattr;
> +};
> +
>  /* List of EVM protected security xattrs */
> -extern char *evm_config_xattrnames[];
> +extern struct list_head evm_hmac_xattrs;
> 
>  int evm_init_key(void);
>  int evm_update_evmxattr(struct dentry *dentry,
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index ab034e5..7e5bfb7 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>  {
>  	struct inode *inode = dentry->d_inode;
>  	struct shash_desc *desc;
> -	char **xattrname;
> +	struct evm_hmac_xattr *entry;
>  	size_t xattr_size = 0;
>  	char *xattr_value = NULL;
>  	int error;
> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>  		return PTR_ERR(desc);
> 
>  	error = -ENODATA;
> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>  		if ((req_xattr_name && req_xattr_value)
> -		    && !strcmp(*xattrname, req_xattr_name)) {
> +		    && !strcmp(entry->xattr, req_xattr_name)) {
>  			error = 0;
>  			crypto_shash_update(desc, (const u8 *)req_xattr_value,
>  					     req_xattr_value_len);
>  			continue;
>  		}
> -		size = vfs_getxattr_alloc(dentry, *xattrname,
> +		size = vfs_getxattr_alloc(dentry, entry->xattr,
>  					  &xattr_value, xattr_size, GFP_NOFS);
>  		if (size == -ENOMEM) {
>  			error = -ENOMEM;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 9c05929..13e03ad 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
>  char *evm_hash = "sha1";
>  int evm_hmac_attrs;
> 
> -char *evm_config_xattrnames[] = {
> -#ifdef CONFIG_SECURITY_SELINUX
> -	XATTR_NAME_SELINUX,
> -#endif
> -#ifdef CONFIG_SECURITY_SMACK
> -	XATTR_NAME_SMACK,
> -#endif
> -#ifdef CONFIG_IMA_APPRAISE
> -	XATTR_NAME_IMA,
> -#endif
> -	XATTR_NAME_CAPS,
> -	NULL
> -};
> +LIST_HEAD(evm_hmac_xattrs);
> 
>  static int evm_fixmode;
>  static int __init evm_set_fixmode(char *str)
> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
>  {
>  	char *attrs = CONFIG_EVM_HMAC_ATTRS;
>  	char *p;
> +	char *xattrs = CONFIG_EVM_HMAC_XATTRS;
> +	struct evm_hmac_xattr *entry;
> 
>  	while ((p = strsep(&attrs, ", \t"))) {
>  		if (!strcmp(p, "fsuuid"))
>  			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
>  	}
>  	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
> +	while ((p = strsep(&xattrs, ", \t"))) {
> +#ifndef CONFIG_SECURITY_SELINUX
> +		if (!strcmp(p, XATTR_NAME_SELINUX))
> +			continue;
> +#endif
> +#ifndef CONFIG_SECURITY_SMACK
> +		if (strstr(p, "SMACK64"))
> +			continue;
> +#endif

As you mentioned, filesystems previously only included
'security.smack64' in the HMAC calculation.  This patch includes all
xattrs prefixed with smack64.  All previously labeled filesystems would
need to be relabeled.

Mimi

> +#ifndef CONFIG_IMA_APPRAISE
> +		if (!strcmp(p, XATTR_NAME_IMA))
> +			continue;
> +#endif
> +		entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> +		if (!entry)
> +			return -ENOMEM;
> +		INIT_LIST_HEAD(&entry->list);
> +		entry->xattr = kstrdup(p, GFP_KERNEL);
> +		if (!entry->xattr)
> +			return -ENOMEM;
> +		list_add_tail(&entry->list, &evm_hmac_xattrs);
> +	}
> +	list_for_each_entry(entry, &evm_hmac_xattrs, list)
> +		pr_info("%s\n", entry->xattr);
>  	return 0;
>  }
> 
>  static int evm_find_protected_xattrs(struct dentry *dentry)
>  {
>  	struct inode *inode = dentry->d_inode;
> -	char **xattr;
> +	struct evm_hmac_xattr *entry;
>  	int error;
>  	int count = 0;
> 
>  	if (!inode->i_op || !inode->i_op->getxattr)
>  		return -EOPNOTSUPP;
> 
> -	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
> -		error = inode->i_op->getxattr(dentry, *xattr, NULL, 0);
> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> +		error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0);
>  		if (error < 0) {
>  			if (error == -ENODATA)
>  				continue;
> @@ -183,19 +197,19 @@ out:
> 
>  static int evm_protected_xattr(const char *req_xattr_name)
>  {
> -	char **xattrname;
> +	struct evm_hmac_xattr *entry;
>  	int namelen;
>  	int found = 0;
> 
>  	namelen = strlen(req_xattr_name);
> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> -		if ((strlen(*xattrname) == namelen)
> -		    && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> +		if ((strlen(entry->xattr) == namelen)
> +		    && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) {
>  			found = 1;
>  			break;
>  		}
>  		if (strncmp(req_xattr_name,
> -			    *xattrname + XATTR_SECURITY_PREFIX_LEN,
> +			    entry->xattr + XATTR_SECURITY_PREFIX_LEN,
>  			    strlen(req_xattr_name)) == 0) {
>  			found = 1;
>  			break;
> @@ -462,19 +476,6 @@ err:
>  	return error;
>  }
> 
> -/*
> - * evm_display_config - list the EVM protected security extended attributes
> - */
> -static int __init evm_display_config(void)
> -{
> -	char **xattrname;
> -
> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
> -		pr_info("%s\n", *xattrname);
> -	return 0;
> -}
> -
> -pure_initcall(evm_display_config);
>  late_initcall(init_evm);
> 
>  MODULE_DESCRIPTION("Extended Verification Module");

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Casey Schaufler]

On 3/3/2014 6:39 PM, Mimi Zohar wrote:
> On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
>> EVM currently uses source hard coded list of xattrs which needs to be
>> included into the HMAC calculation. This is very unflexible.
>> Adding new attributes requires modifcation of the source code and
>> prevents building the kernel which works with previously labeled
>> filesystems.
>>
>> Early versions of Smack used only one xattr security.SMACK64,
>> which is protected by EVM. Now Smack has a few more attributes and
>> they are not protected. Adding support to the source code makes it
>> impossible to use new kernel with previousely labeled filesystems.
> I think this patch will break any existing filesystems labeled with
> 'security.smack64'.  Details inline.
>
>> This patch replaces hardcoded xattr array with dynamic list which is
>> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
>> kernel with with support of older and newer EVM HMAC formats.
>>
>> Possible future extension will be to read xattr list from the kernel
>> command line or from securityfs entry.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  security/integrity/evm/Kconfig      | 10 ++++++
>>  security/integrity/evm/evm.h        |  7 +++-
>>  security/integrity/evm/evm_crypto.c |  8 ++---
>>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
>>  4 files changed, 55 insertions(+), 39 deletions(-)
>>
>> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
>> index 2be51fa..06237b8 100644
>> --- a/security/integrity/evm/Kconfig
>> +++ b/security/integrity/evm/Kconfig
>> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
>>  	  WARNING: changing the HMAC calculation method or adding 
>>  	  additional info to the calculation, requires existing EVM
>>  	  labeled file systems to be relabeled.
>> +
>> +config EVM_HMAC_XATTRS
>> +	string "HMAC xattrs"
>> +	default "security.selinux security.SMACK64 security.ima security.capability"
>> +	help
>> +	  This options allows to specify list of extended attributes included into HMAC
>> +	  calculation. It makes it possible easily upgrade to newer kernels.
>> +
>> +	  Default value:
>> +	    security.selinux, security.SMACK64, security.ima, security.capability
>> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> index c8fa0aa..4d1c51e 100644
>> --- a/security/integrity/evm/evm.h
>> +++ b/security/integrity/evm/evm.h
>> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
>>
>>  #define EVM_HMAC_ATTR_FSUUID		0x0001
>>
>> +struct evm_hmac_xattr {
>> +	struct list_head list;
>> +	char *xattr;
>> +};
>> +
>>  /* List of EVM protected security xattrs */
>> -extern char *evm_config_xattrnames[];
>> +extern struct list_head evm_hmac_xattrs;
>>
>>  int evm_init_key(void);
>>  int evm_update_evmxattr(struct dentry *dentry,
>> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> index ab034e5..7e5bfb7 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>>  {
>>  	struct inode *inode = dentry->d_inode;
>>  	struct shash_desc *desc;
>> -	char **xattrname;
>> +	struct evm_hmac_xattr *entry;
>>  	size_t xattr_size = 0;
>>  	char *xattr_value = NULL;
>>  	int error;
>> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>>  		return PTR_ERR(desc);
>>
>>  	error = -ENODATA;
>> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>>  		if ((req_xattr_name && req_xattr_value)
>> -		    && !strcmp(*xattrname, req_xattr_name)) {
>> +		    && !strcmp(entry->xattr, req_xattr_name)) {
>>  			error = 0;
>>  			crypto_shash_update(desc, (const u8 *)req_xattr_value,
>>  					     req_xattr_value_len);
>>  			continue;
>>  		}
>> -		size = vfs_getxattr_alloc(dentry, *xattrname,
>> +		size = vfs_getxattr_alloc(dentry, entry->xattr,
>>  					  &xattr_value, xattr_size, GFP_NOFS);
>>  		if (size == -ENOMEM) {
>>  			error = -ENOMEM;
>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> index 9c05929..13e03ad 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
>>  char *evm_hash = "sha1";
>>  int evm_hmac_attrs;
>>
>> -char *evm_config_xattrnames[] = {
>> -#ifdef CONFIG_SECURITY_SELINUX
>> -	XATTR_NAME_SELINUX,
>> -#endif
>> -#ifdef CONFIG_SECURITY_SMACK
>> -	XATTR_NAME_SMACK,
>> -#endif
>> -#ifdef CONFIG_IMA_APPRAISE
>> -	XATTR_NAME_IMA,
>> -#endif
>> -	XATTR_NAME_CAPS,
>> -	NULL
>> -};
>> +LIST_HEAD(evm_hmac_xattrs);
>>
>>  static int evm_fixmode;
>>  static int __init evm_set_fixmode(char *str)
>> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
>>  {
>>  	char *attrs = CONFIG_EVM_HMAC_ATTRS;
>>  	char *p;
>> +	char *xattrs = CONFIG_EVM_HMAC_XATTRS;
>> +	struct evm_hmac_xattr *entry;
>>
>>  	while ((p = strsep(&attrs, ", \t"))) {
>>  		if (!strcmp(p, "fsuuid"))
>>  			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
>>  	}
>>  	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
>> +	while ((p = strsep(&xattrs, ", \t"))) {
>> +#ifndef CONFIG_SECURITY_SELINUX
>> +		if (!strcmp(p, XATTR_NAME_SELINUX))
>> +			continue;
>> +#endif
>> +#ifndef CONFIG_SECURITY_SMACK
>> +		if (strstr(p, "SMACK64"))
>> +			continue;
>> +#endif
> As you mentioned, filesystems previously only included
> 'security.smack64' in the HMAC calculation.  This patch includes all
> xattrs prefixed with smack64.  All previously labeled filesystems would
> need to be relabeled.
>
> Mimi

Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC
and SMACK64TRANSMUTE attributes are optional. You do not want these
attributes in most cases. Few files should actually have them. They
should only be used if they are there, and ignored otherwise. If that
is not possible it is better to ignore them completely. 

>
>> +#ifndef CONFIG_IMA_APPRAISE
>> +		if (!strcmp(p, XATTR_NAME_IMA))
>> +			continue;
>> +#endif
>> +		entry = kmalloc(sizeof(*entry), GFP_KERNEL);
>> +		if (!entry)
>> +			return -ENOMEM;
>> +		INIT_LIST_HEAD(&entry->list);
>> +		entry->xattr = kstrdup(p, GFP_KERNEL);
>> +		if (!entry->xattr)
>> +			return -ENOMEM;
>> +		list_add_tail(&entry->list, &evm_hmac_xattrs);
>> +	}
>> +	list_for_each_entry(entry, &evm_hmac_xattrs, list)
>> +		pr_info("%s\n", entry->xattr);
>>  	return 0;
>>  }
>>
>>  static int evm_find_protected_xattrs(struct dentry *dentry)
>>  {
>>  	struct inode *inode = dentry->d_inode;
>> -	char **xattr;
>> +	struct evm_hmac_xattr *entry;
>>  	int error;
>>  	int count = 0;
>>
>>  	if (!inode->i_op || !inode->i_op->getxattr)
>>  		return -EOPNOTSUPP;
>>
>> -	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
>> -		error = inode->i_op->getxattr(dentry, *xattr, NULL, 0);
>> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> +		error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0);
>>  		if (error < 0) {
>>  			if (error == -ENODATA)
>>  				continue;
>> @@ -183,19 +197,19 @@ out:
>>
>>  static int evm_protected_xattr(const char *req_xattr_name)
>>  {
>> -	char **xattrname;
>> +	struct evm_hmac_xattr *entry;
>>  	int namelen;
>>  	int found = 0;
>>
>>  	namelen = strlen(req_xattr_name);
>> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>> -		if ((strlen(*xattrname) == namelen)
>> -		    && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
>> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> +		if ((strlen(entry->xattr) == namelen)
>> +		    && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) {
>>  			found = 1;
>>  			break;
>>  		}
>>  		if (strncmp(req_xattr_name,
>> -			    *xattrname + XATTR_SECURITY_PREFIX_LEN,
>> +			    entry->xattr + XATTR_SECURITY_PREFIX_LEN,
>>  			    strlen(req_xattr_name)) == 0) {
>>  			found = 1;
>>  			break;
>> @@ -462,19 +476,6 @@ err:
>>  	return error;
>>  }
>>
>> -/*
>> - * evm_display_config - list the EVM protected security extended attributes
>> - */
>> -static int __init evm_display_config(void)
>> -{
>> -	char **xattrname;
>> -
>> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
>> -		pr_info("%s\n", *xattrname);
>> -	return 0;
>> -}
>> -
>> -pure_initcall(evm_display_config);
>>  late_initcall(init_evm);
>>
>>  MODULE_DESCRIPTION("Extended Verification Module");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Mimi Zohar]

On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: 
> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: 
> >> EVM currently uses source hard coded list of xattrs which needs to be
> >> included into the HMAC calculation. This is very unflexible.
> >> Adding new attributes requires modifcation of the source code and
> >> prevents building the kernel which works with previously labeled
> >> filesystems.
> >>
> >> Early versions of Smack used only one xattr security.SMACK64,
> >> which is protected by EVM. Now Smack has a few more attributes and
> >> they are not protected. Adding support to the source code makes it
> >> impossible to use new kernel with previousely labeled filesystems.
> > I think this patch will break any existing filesystems labeled with
> > 'security.smack64'.  Details inline.
> >
> >> This patch replaces hardcoded xattr array with dynamic list which is
> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
> >> kernel with with support of older and newer EVM HMAC formats.
> >>
> >> Possible future extension will be to read xattr list from the kernel
> >> command line or from securityfs entry.
> >>
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >> ---
> >>  security/integrity/evm/Kconfig      | 10 ++++++
> >>  security/integrity/evm/evm.h        |  7 +++-
> >>  security/integrity/evm/evm_crypto.c |  8 ++---
> >>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
> >>  4 files changed, 55 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> >> index 2be51fa..06237b8 100644
> >> --- a/security/integrity/evm/Kconfig
> >> +++ b/security/integrity/evm/Kconfig
> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
> >>  	  WARNING: changing the HMAC calculation method or adding 
> >>  	  additional info to the calculation, requires existing EVM
> >>  	  labeled file systems to be relabeled.
> >> +
> >> +config EVM_HMAC_XATTRS
> >> +	string "HMAC xattrs"
> >> +	default "security.selinux security.SMACK64 security.ima security.capability"
> >> +	help
> >> +	  This options allows to specify list of extended attributes included into HMAC
> >> +	  calculation. It makes it possible easily upgrade to newer kernels.
> >> +
> >> +	  Default value:
> >> +	    security.selinux, security.SMACK64, security.ima, security.capability
> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> >> index c8fa0aa..4d1c51e 100644
> >> --- a/security/integrity/evm/evm.h
> >> +++ b/security/integrity/evm/evm.h
> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
> >>
> >>  #define EVM_HMAC_ATTR_FSUUID		0x0001
> >>
> >> +struct evm_hmac_xattr {
> >> +	struct list_head list;
> >> +	char *xattr;
> >> +};
> >> +
> >>  /* List of EVM protected security xattrs */
> >> -extern char *evm_config_xattrnames[];
> >> +extern struct list_head evm_hmac_xattrs;
> >>
> >>  int evm_init_key(void);
> >>  int evm_update_evmxattr(struct dentry *dentry,
> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> >> index ab034e5..7e5bfb7 100644
> >> --- a/security/integrity/evm/evm_crypto.c
> >> +++ b/security/integrity/evm/evm_crypto.c
> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> >>  {
> >>  	struct inode *inode = dentry->d_inode;
> >>  	struct shash_desc *desc;
> >> -	char **xattrname;
> >> +	struct evm_hmac_xattr *entry;
> >>  	size_t xattr_size = 0;
> >>  	char *xattr_value = NULL;
> >>  	int error;
> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> >>  		return PTR_ERR(desc);
> >>
> >>  	error = -ENODATA;
> >> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> >> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> >>  		if ((req_xattr_name && req_xattr_value)
> >> -		    && !strcmp(*xattrname, req_xattr_name)) {
> >> +		    && !strcmp(entry->xattr, req_xattr_name)) {
> >>  			error = 0;
> >>  			crypto_shash_update(desc, (const u8 *)req_xattr_value,
> >>  					     req_xattr_value_len);
> >>  			continue;
> >>  		}
> >> -		size = vfs_getxattr_alloc(dentry, *xattrname,
> >> +		size = vfs_getxattr_alloc(dentry, entry->xattr,
> >>  					  &xattr_value, xattr_size, GFP_NOFS);
> >>  		if (size == -ENOMEM) {
> >>  			error = -ENOMEM;
> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> >> index 9c05929..13e03ad 100644
> >> --- a/security/integrity/evm/evm_main.c
> >> +++ b/security/integrity/evm/evm_main.c
> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
> >>  char *evm_hash = "sha1";
> >>  int evm_hmac_attrs;
> >>
> >> -char *evm_config_xattrnames[] = {
> >> -#ifdef CONFIG_SECURITY_SELINUX
> >> -	XATTR_NAME_SELINUX,
> >> -#endif
> >> -#ifdef CONFIG_SECURITY_SMACK
> >> -	XATTR_NAME_SMACK,
> >> -#endif
> >> -#ifdef CONFIG_IMA_APPRAISE
> >> -	XATTR_NAME_IMA,
> >> -#endif
> >> -	XATTR_NAME_CAPS,
> >> -	NULL
> >> -};
> >> +LIST_HEAD(evm_hmac_xattrs);
> >>
> >>  static int evm_fixmode;
> >>  static int __init evm_set_fixmode(char *str)
> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
> >>  {
> >>  	char *attrs = CONFIG_EVM_HMAC_ATTRS;
> >>  	char *p;
> >> +	char *xattrs = CONFIG_EVM_HMAC_XATTRS;
> >> +	struct evm_hmac_xattr *entry;
> >>
> >>  	while ((p = strsep(&attrs, ", \t"))) {
> >>  		if (!strcmp(p, "fsuuid"))
> >>  			evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
> >>  	}
> >>  	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
> >> +	while ((p = strsep(&xattrs, ", \t"))) {
> >> +#ifndef CONFIG_SECURITY_SELINUX
> >> +		if (!strcmp(p, XATTR_NAME_SELINUX))
> >> +			continue;
> >> +#endif
> >> +#ifndef CONFIG_SECURITY_SMACK
> >> +		if (strstr(p, "SMACK64"))
> >> +			continue;
> >> +#endif
> > As you mentioned, filesystems previously only included
> > 'security.smack64' in the HMAC calculation.  This patch includes all
> > xattrs prefixed with smack64.  All previously labeled filesystems would
> > need to be relabeled.
> >
> > Mimi
> 
> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC
> and SMACK64TRANSMUTE attributes are optional. You do not want these
> attributes in most cases. Few files should actually have them. They
> should only be used if they are there, and ignored otherwise. If that
> is not possible it is better to ignore them completely. 

Going forward the code would work just fine.  All smack64 prefixed
labels would be included in the HMAC calculation.  My concern is for
existing labeled filesystems, which only included the SMACK64 label in
the HMAC calculation, but have other SMACK64 labels.  The HMAC
verification would fail for these files.

Mimi

> >
> >> +#ifndef CONFIG_IMA_APPRAISE
> >> +		if (!strcmp(p, XATTR_NAME_IMA))
> >> +			continue;
> >> +#endif
> >> +		entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> >> +		if (!entry)
> >> +			return -ENOMEM;
> >> +		INIT_LIST_HEAD(&entry->list);
> >> +		entry->xattr = kstrdup(p, GFP_KERNEL);
> >> +		if (!entry->xattr)
> >> +			return -ENOMEM;
> >> +		list_add_tail(&entry->list, &evm_hmac_xattrs);
> >> +	}
> >> +	list_for_each_entry(entry, &evm_hmac_xattrs, list)
> >> +		pr_info("%s\n", entry->xattr);
> >>  	return 0;
> >>  }
> >>
> >>  static int evm_find_protected_xattrs(struct dentry *dentry)
> >>  {
> >>  	struct inode *inode = dentry->d_inode;
> >> -	char **xattr;
> >> +	struct evm_hmac_xattr *entry;
> >>  	int error;
> >>  	int count = 0;
> >>
> >>  	if (!inode->i_op || !inode->i_op->getxattr)
> >>  		return -EOPNOTSUPP;
> >>
> >> -	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
> >> -		error = inode->i_op->getxattr(dentry, *xattr, NULL, 0);
> >> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> >> +		error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0);
> >>  		if (error < 0) {
> >>  			if (error == -ENODATA)
> >>  				continue;
> >> @@ -183,19 +197,19 @@ out:
> >>
> >>  static int evm_protected_xattr(const char *req_xattr_name)
> >>  {
> >> -	char **xattrname;
> >> +	struct evm_hmac_xattr *entry;
> >>  	int namelen;
> >>  	int found = 0;
> >>
> >>  	namelen = strlen(req_xattr_name);
> >> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> >> -		if ((strlen(*xattrname) == namelen)
> >> -		    && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
> >> +	list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> >> +		if ((strlen(entry->xattr) == namelen)
> >> +		    && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) {
> >>  			found = 1;
> >>  			break;
> >>  		}
> >>  		if (strncmp(req_xattr_name,
> >> -			    *xattrname + XATTR_SECURITY_PREFIX_LEN,
> >> +			    entry->xattr + XATTR_SECURITY_PREFIX_LEN,
> >>  			    strlen(req_xattr_name)) == 0) {
> >>  			found = 1;
> >>  			break;
> >> @@ -462,19 +476,6 @@ err:
> >>  	return error;
> >>  }
> >>
> >> -/*
> >> - * evm_display_config - list the EVM protected security extended attributes
> >> - */
> >> -static int __init evm_display_config(void)
> >> -{
> >> -	char **xattrname;
> >> -
> >> -	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
> >> -		pr_info("%s\n", *xattrname);
> >> -	return 0;
> >> -}
> >> -
> >> -pure_initcall(evm_display_config);
> >>  late_initcall(init_evm);
> >>
> >>  MODULE_DESCRIPTION("Extended Verification Module");
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
> 

Re: [PATCH 6/8] evm: enable key retention service automatically

[Dmitry Kasatkin]

On Tue, Mar 4, 2014 at 4:02 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> If keys are not enabled, EVM is not visible in the configuration menu.
>> It may be difficult to figure out what to do unless  you really know.
>>
>> Other subsystems as NFS, CIFS select keys automatically.
>> This patch does the same.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  security/integrity/evm/Kconfig | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
>> index 5aa9103..d35b491 100644
>> --- a/security/integrity/evm/Kconfig
>> +++ b/security/integrity/evm/Kconfig
>> @@ -1,9 +1,10 @@
>>  config EVM
>>       boolean "EVM support"
>> -     depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)
>
> Including KEYS is fine, but the trusted-keys dependency is still
> required.  If trusted-keys is enabled, then the TPM and trusted-keys
> must be builtin.
>

Hello.

EVM does not depend on trusted keys, but on encrypted keys.
There is no need for "&& (TRUSTED_KEYS=y || TRUSTED_KEYS=n)"

Header file already provides all necessary compile time dependencies...

-------------------------------------------------------------------------------------------------
#if defined(CONFIG_TRUSTED_KEYS) || \
  (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE))
extern struct key *request_trusted_key(const char *trusted_desc,
      u8 **master_key, size_t *master_keylen);
#else
static inline struct key *request_trusted_key(const char *trusted_desc,
     u8 **master_key,
     size_t *master_keylen)
{
return ERR_PTR(-EOPNOTSUPP);
}
#endif
-------------------------------------------------------------------------------------------------


- Dmitry

> Mimi
>
>> +     depends on SECURITY
>> +     select KEYS
>> +     select ENCRYPTED_KEYS
>>       select CRYPTO_HMAC
>>       select CRYPTO_SHA1
>> -     select ENCRYPTED_KEYS
>>       default n
>>       help
>>         EVM protects a file's security extended attributes against
>
>



-- 
Thanks,
Dmitry

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Dmitry Kasatkin]

On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote:
>> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
>> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> >> EVM currently uses source hard coded list of xattrs which needs to be
>> >> included into the HMAC calculation. This is very unflexible.
>> >> Adding new attributes requires modifcation of the source code and
>> >> prevents building the kernel which works with previously labeled
>> >> filesystems.
>> >>
>> >> Early versions of Smack used only one xattr security.SMACK64,
>> >> which is protected by EVM. Now Smack has a few more attributes and
>> >> they are not protected. Adding support to the source code makes it
>> >> impossible to use new kernel with previousely labeled filesystems.
>> > I think this patch will break any existing filesystems labeled with
>> > 'security.smack64'.  Details inline.
>> >
>> >> This patch replaces hardcoded xattr array with dynamic list which is
>> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
>> >> kernel with with support of older and newer EVM HMAC formats.
>> >>
>> >> Possible future extension will be to read xattr list from the kernel
>> >> command line or from securityfs entry.
>> >>
>> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> >> ---
>> >>  security/integrity/evm/Kconfig      | 10 ++++++
>> >>  security/integrity/evm/evm.h        |  7 +++-
>> >>  security/integrity/evm/evm_crypto.c |  8 ++---
>> >>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
>> >>  4 files changed, 55 insertions(+), 39 deletions(-)
>> >>
>> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
>> >> index 2be51fa..06237b8 100644
>> >> --- a/security/integrity/evm/Kconfig
>> >> +++ b/security/integrity/evm/Kconfig
>> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
>> >>      WARNING: changing the HMAC calculation method or adding
>> >>      additional info to the calculation, requires existing EVM
>> >>      labeled file systems to be relabeled.
>> >> +
>> >> +config EVM_HMAC_XATTRS
>> >> +  string "HMAC xattrs"
>> >> +  default "security.selinux security.SMACK64 security.ima security.capability"
>> >> +  help
>> >> +    This options allows to specify list of extended attributes included into HMAC
>> >> +    calculation. It makes it possible easily upgrade to newer kernels.
>> >> +
>> >> +    Default value:
>> >> +      security.selinux, security.SMACK64, security.ima, security.capability
>> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> >> index c8fa0aa..4d1c51e 100644
>> >> --- a/security/integrity/evm/evm.h
>> >> +++ b/security/integrity/evm/evm.h
>> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
>> >>
>> >>  #define EVM_HMAC_ATTR_FSUUID              0x0001
>> >>
>> >> +struct evm_hmac_xattr {
>> >> +  struct list_head list;
>> >> +  char *xattr;
>> >> +};
>> >> +
>> >>  /* List of EVM protected security xattrs */
>> >> -extern char *evm_config_xattrnames[];
>> >> +extern struct list_head evm_hmac_xattrs;
>> >>
>> >>  int evm_init_key(void);
>> >>  int evm_update_evmxattr(struct dentry *dentry,
>> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> >> index ab034e5..7e5bfb7 100644
>> >> --- a/security/integrity/evm/evm_crypto.c
>> >> +++ b/security/integrity/evm/evm_crypto.c
>> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>> >>  {
>> >>    struct inode *inode = dentry->d_inode;
>> >>    struct shash_desc *desc;
>> >> -  char **xattrname;
>> >> +  struct evm_hmac_xattr *entry;
>> >>    size_t xattr_size = 0;
>> >>    char *xattr_value = NULL;
>> >>    int error;
>> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>> >>            return PTR_ERR(desc);
>> >>
>> >>    error = -ENODATA;
>> >> -  for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> >>            if ((req_xattr_name && req_xattr_value)
>> >> -              && !strcmp(*xattrname, req_xattr_name)) {
>> >> +              && !strcmp(entry->xattr, req_xattr_name)) {
>> >>                    error = 0;
>> >>                    crypto_shash_update(desc, (const u8 *)req_xattr_value,
>> >>                                         req_xattr_value_len);
>> >>                    continue;
>> >>            }
>> >> -          size = vfs_getxattr_alloc(dentry, *xattrname,
>> >> +          size = vfs_getxattr_alloc(dentry, entry->xattr,
>> >>                                      &xattr_value, xattr_size, GFP_NOFS);
>> >>            if (size == -ENOMEM) {
>> >>                    error = -ENOMEM;
>> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> >> index 9c05929..13e03ad 100644
>> >> --- a/security/integrity/evm/evm_main.c
>> >> +++ b/security/integrity/evm/evm_main.c
>> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
>> >>  char *evm_hash = "sha1";
>> >>  int evm_hmac_attrs;
>> >>
>> >> -char *evm_config_xattrnames[] = {
>> >> -#ifdef CONFIG_SECURITY_SELINUX
>> >> -  XATTR_NAME_SELINUX,
>> >> -#endif
>> >> -#ifdef CONFIG_SECURITY_SMACK
>> >> -  XATTR_NAME_SMACK,
>> >> -#endif
>> >> -#ifdef CONFIG_IMA_APPRAISE
>> >> -  XATTR_NAME_IMA,
>> >> -#endif
>> >> -  XATTR_NAME_CAPS,
>> >> -  NULL
>> >> -};
>> >> +LIST_HEAD(evm_hmac_xattrs);
>> >>
>> >>  static int evm_fixmode;
>> >>  static int __init evm_set_fixmode(char *str)
>> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
>> >>  {
>> >>    char *attrs = CONFIG_EVM_HMAC_ATTRS;
>> >>    char *p;
>> >> +  char *xattrs = CONFIG_EVM_HMAC_XATTRS;
>> >> +  struct evm_hmac_xattr *entry;
>> >>
>> >>    while ((p = strsep(&attrs, ", \t"))) {
>> >>            if (!strcmp(p, "fsuuid"))
>> >>                    evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
>> >>    }
>> >>    pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
>> >> +  while ((p = strsep(&xattrs, ", \t"))) {
>> >> +#ifndef CONFIG_SECURITY_SELINUX
>> >> +          if (!strcmp(p, XATTR_NAME_SELINUX))
>> >> +                  continue;
>> >> +#endif
>> >> +#ifndef CONFIG_SECURITY_SMACK
>> >> +          if (strstr(p, "SMACK64"))
>> >> +                  continue;
>> >> +#endif
>> > As you mentioned, filesystems previously only included
>> > 'security.smack64' in the HMAC calculation.  This patch includes all
>> > xattrs prefixed with smack64.  All previously labeled filesystems would
>> > need to be relabeled.
>> >
>> > Mimi
>>
>> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC
>> and SMACK64TRANSMUTE attributes are optional. You do not want these
>> attributes in most cases. Few files should actually have them. They
>> should only be used if they are there, and ignored otherwise. If that
>> is not possible it is better to ignore them completely.
>
> Going forward the code would work just fine.  All smack64 prefixed
> labels would be included in the HMAC calculation.  My concern is for
> existing labeled filesystems, which only included the SMACK64 label in
> the HMAC calculation, but have other SMACK64 labels.  The HMAC
> verification would fail for these files.
>

Hello,

There is no problem for existing labeled filesystems.

Patch does not work with SMACK64 prefix, but with exact name.

Only xattrs listed in the configuration options are included
By default "security.selinux, security.SMACK64, security.ima,
security.capability"
which correspond to EVM current functionality...

Existing systems using new kernel may stick to original EVM xattr set.

But new systems might prefer to add "more" xattrs..

This patch makes it flexible...

- Dmitry


> Mimi
>
>> >
>> >> +#ifndef CONFIG_IMA_APPRAISE
>> >> +          if (!strcmp(p, XATTR_NAME_IMA))
>> >> +                  continue;
>> >> +#endif
>> >> +          entry = kmalloc(sizeof(*entry), GFP_KERNEL);
>> >> +          if (!entry)
>> >> +                  return -ENOMEM;
>> >> +          INIT_LIST_HEAD(&entry->list);
>> >> +          entry->xattr = kstrdup(p, GFP_KERNEL);
>> >> +          if (!entry->xattr)
>> >> +                  return -ENOMEM;
>> >> +          list_add_tail(&entry->list, &evm_hmac_xattrs);
>> >> +  }
>> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list)
>> >> +          pr_info("%s\n", entry->xattr);
>> >>    return 0;
>> >>  }
>> >>
>> >>  static int evm_find_protected_xattrs(struct dentry *dentry)
>> >>  {
>> >>    struct inode *inode = dentry->d_inode;
>> >> -  char **xattr;
>> >> +  struct evm_hmac_xattr *entry;
>> >>    int error;
>> >>    int count = 0;
>> >>
>> >>    if (!inode->i_op || !inode->i_op->getxattr)
>> >>            return -EOPNOTSUPP;
>> >>
>> >> -  for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
>> >> -          error = inode->i_op->getxattr(dentry, *xattr, NULL, 0);
>> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> >> +          error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0);
>> >>            if (error < 0) {
>> >>                    if (error == -ENODATA)
>> >>                            continue;
>> >> @@ -183,19 +197,19 @@ out:
>> >>
>> >>  static int evm_protected_xattr(const char *req_xattr_name)
>> >>  {
>> >> -  char **xattrname;
>> >> +  struct evm_hmac_xattr *entry;
>> >>    int namelen;
>> >>    int found = 0;
>> >>
>> >>    namelen = strlen(req_xattr_name);
>> >> -  for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>> >> -          if ((strlen(*xattrname) == namelen)
>> >> -              && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
>> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> >> +          if ((strlen(entry->xattr) == namelen)
>> >> +              && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) {
>> >>                    found = 1;
>> >>                    break;
>> >>            }
>> >>            if (strncmp(req_xattr_name,
>> >> -                      *xattrname + XATTR_SECURITY_PREFIX_LEN,
>> >> +                      entry->xattr + XATTR_SECURITY_PREFIX_LEN,
>> >>                        strlen(req_xattr_name)) == 0) {
>> >>                    found = 1;
>> >>                    break;
>> >> @@ -462,19 +476,6 @@ err:
>> >>    return error;
>> >>  }
>> >>
>> >> -/*
>> >> - * evm_display_config - list the EVM protected security extended attributes
>> >> - */
>> >> -static int __init evm_display_config(void)
>> >> -{
>> >> -  char **xattrname;
>> >> -
>> >> -  for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++)
>> >> -          pr_info("%s\n", *xattrname);
>> >> -  return 0;
>> >> -}
>> >> -
>> >> -pure_initcall(evm_display_config);
>> >>  late_initcall(init_evm);
>> >>
>> >>  MODULE_DESCRIPTION("Extended Verification Module");
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at  http://www.tux.org/lkml/
>> >
>>
>
>



-- 
Thanks,
Dmitry

Re: [PATCH 7/8] evm: introduce EVM hmac attribute list

[Dmitry Kasatkin]

On Tue, Mar 4, 2014 at 4:09 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> This patch replaces using of hmac version configuration parameter
>> with attribute list. It allows to build kernels which works with
>> previously labeled filesystems.
>>
>> Currently supported attribute is 'fsuuid' which is equivalent of
>> former version 2.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>
> Please include the new boot command line option in
> Documentation/kernel-parameters.txt.
>

There is no kernel parameter, but configuration parameter..
Again for flexibility to add more parameters and be able to use new
kernel on existing labeled filesystems.

Kernel command line option can be added on the future.

- Dmitry

> Mimi
>
>> ---
>>  security/integrity/evm/Kconfig      | 19 ++++++++++---------
>>  security/integrity/evm/evm.h        |  4 +++-
>>  security/integrity/evm/evm_crypto.c |  2 +-
>>  security/integrity/evm/evm_main.c   | 21 ++++++++++++++++++++-
>>  4 files changed, 34 insertions(+), 12 deletions(-)
>>
>> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
>> index d35b491..2be51fa 100644
>> --- a/security/integrity/evm/Kconfig
>> +++ b/security/integrity/evm/Kconfig
>> @@ -12,15 +12,16 @@ config EVM
>>
>>         If you are unsure how to answer this question, answer N.
>>
>> -config EVM_HMAC_VERSION
>> -     int "EVM HMAC version"
>> -     depends on EVM
>> -     default 2
>> -     help
>> -       This options adds EVM HMAC version support.
>> -       1 - original version
>> -       2 - add per filesystem unique identifier (UUID) (default)
>> +config EVM_HMAC_ATTRS
>> +     string "HMAC attributes"
>> +     default "fsuuid"
>> +     help
>> +       This options allows to specify list of optional attributes included into HMAC
>> +       calculation. It makes it possible easily upgrade to newer kernels.
>> +
>> +       Default value is 'fsuuid', which is former version 2.
>> +       if blank, it is equivalent of version 1
>>
>>         WARNING: changing the HMAC calculation method or adding
>>         additional info to the calculation, requires existing EVM
>> -       labeled file systems to be relabeled.
>> +       labeled file systems to be relabeled.
>> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> index 37c88dd..c8fa0aa 100644
>> --- a/security/integrity/evm/evm.h
>> +++ b/security/integrity/evm/evm.h
>> @@ -24,11 +24,13 @@
>>  extern int evm_initialized;
>>  extern char *evm_hmac;
>>  extern char *evm_hash;
>> -extern int evm_hmac_version;
>> +extern int evm_hmac_attrs;
>>
>>  extern struct crypto_shash *hmac_tfm;
>>  extern struct crypto_shash *hash_tfm;
>>
>> +#define EVM_HMAC_ATTR_FSUUID         0x0001
>> +
>>  /* List of EVM protected security xattrs */
>>  extern char *evm_config_xattrnames[];
>>
>> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> index babd862..ab034e5 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>>       hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
>>       hmac_misc.mode = inode->i_mode;
>>       crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
>> -     if (evm_hmac_version > 1)
>> +     if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID)
>>               crypto_shash_update(desc, inode->i_sb->s_uuid,
>>                                   sizeof(inode->i_sb->s_uuid));
>>       crypto_shash_final(desc, digest);
>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> index 996092f..9c05929 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -32,7 +32,7 @@ static char *integrity_status_msg[] = {
>>  };
>>  char *evm_hmac = "hmac(sha1)";
>>  char *evm_hash = "sha1";
>> -int evm_hmac_version = CONFIG_EVM_HMAC_VERSION;
>> +int evm_hmac_attrs;
>>
>>  char *evm_config_xattrnames[] = {
>>  #ifdef CONFIG_SECURITY_SELINUX
>> @@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str)
>>  }
>>  __setup("evm=", evm_set_fixmode);
>>
>> +static int __init evm_init_config(void)
>> +{
>> +     char *attrs = CONFIG_EVM_HMAC_ATTRS;
>> +     char *p;
>> +
>> +     while ((p = strsep(&attrs, ", \t"))) {
>> +             if (!strcmp(p, "fsuuid"))
>> +                     evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
>> +     }
>> +     pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
>> +     return 0;
>> +}
>> +
>>  static int evm_find_protected_xattrs(struct dentry *dentry)
>>  {
>>       struct inode *inode = dentry->d_inode;
>> @@ -432,6 +445,12 @@ static int __init init_evm(void)
>>  {
>>       int error;
>>
>> +     error = evm_init_config();
>> +     if (error < 0) {
>> +             pr_info("Error parsing config lists\n");
>> +             goto err;
>> +     }
>> +
>>       error = evm_init_secfs();
>>       if (error < 0) {
>>               pr_info("Error registering secfs\n");
>
>



-- 
Thanks,
Dmitry

Re: [PATCH 6/8] evm: enable key retention service automatically

[Mimi Zohar]

On Tue, 2014-03-04 at 16:10 +0200, Dmitry Kasatkin wrote: 
> On Tue, Mar 4, 2014 at 4:02 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
> >> If keys are not enabled, EVM is not visible in the configuration menu.
> >> It may be difficult to figure out what to do unless  you really know.
> >>
> >> Other subsystems as NFS, CIFS select keys automatically.
> >> This patch does the same.
> >>
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >> ---
> >>  security/integrity/evm/Kconfig | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> >> index 5aa9103..d35b491 100644
> >> --- a/security/integrity/evm/Kconfig
> >> +++ b/security/integrity/evm/Kconfig
> >> @@ -1,9 +1,10 @@
> >>  config EVM
> >>       boolean "EVM support"
> >> -     depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n)
> >
> > Including KEYS is fine, but the trusted-keys dependency is still
> > required.  If trusted-keys is enabled, then the TPM and trusted-keys
> > must be builtin.
> >
> 
> Hello.
> 
> EVM does not depend on trusted keys, but on encrypted keys.
> There is no need for "&& (TRUSTED_KEYS=y || TRUSTED_KEYS=n)"
> 
> Header file already provides all necessary compile time dependencies...
> 
> -------------------------------------------------------------------------------------------------
> #if defined(CONFIG_TRUSTED_KEYS) || \
>   (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE))
> extern struct key *request_trusted_key(const char *trusted_desc,
>       u8 **master_key, size_t *master_keylen);
> #else
> static inline struct key *request_trusted_key(const char *trusted_desc,
>      u8 **master_key,
>      size_t *master_keylen)
> {
> return ERR_PTR(-EOPNOTSUPP);
> }
> #endif
> -------------------------------------------------------------------------------------------------

Either in the patch description or the changelog, please note the reason
for removing the builtin TRUSTED_KEY dependency.  

thanks,

Mimi

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Mimi Zohar]

On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: 
> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote:
> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
> >> >> EVM currently uses source hard coded list of xattrs which needs to be
> >> >> included into the HMAC calculation. This is very unflexible.
> >> >> Adding new attributes requires modifcation of the source code and
> >> >> prevents building the kernel which works with previously labeled
> >> >> filesystems.
> >> >>
> >> >> Early versions of Smack used only one xattr security.SMACK64,
> >> >> which is protected by EVM. Now Smack has a few more attributes and
> >> >> they are not protected. Adding support to the source code makes it
> >> >> impossible to use new kernel with previousely labeled filesystems.
> >> > I think this patch will break any existing filesystems labeled with
> >> > 'security.smack64'.  Details inline.
> >> >
> >> >> This patch replaces hardcoded xattr array with dynamic list which is
> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
> >> >> kernel with with support of older and newer EVM HMAC formats.

So instead of having a single kernel, this allows you to build different
kernels with different xattr labels included in the HMAC.  Wouldn't you
want a migration mode, similar to 'fix' mode, that only updates the
HMAC, if the existing HMAC verified based on the prior set of xattrs?

> >> >>
> >> >> Possible future extension will be to read xattr list from the kernel
> >> >> command line or from securityfs entry.
> >> >>
> >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >> >> ---
> >> >>  security/integrity/evm/Kconfig      | 10 ++++++
> >> >>  security/integrity/evm/evm.h        |  7 +++-
> >> >>  security/integrity/evm/evm_crypto.c |  8 ++---
> >> >>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
> >> >>  4 files changed, 55 insertions(+), 39 deletions(-)
> >> >>
> >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
> >> >> index 2be51fa..06237b8 100644
> >> >> --- a/security/integrity/evm/Kconfig
> >> >> +++ b/security/integrity/evm/Kconfig
> >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
> >> >>      WARNING: changing the HMAC calculation method or adding
> >> >>      additional info to the calculation, requires existing EVM
> >> >>      labeled file systems to be relabeled.
> >> >> +
> >> >> +config EVM_HMAC_XATTRS
> >> >> +  string "HMAC xattrs"
> >> >> +  default "security.selinux security.SMACK64 security.ima security.capability"
> >> >> +  help
> >> >> +    This options allows to specify list of extended attributes included into HMAC
> >> >> +    calculation. It makes it possible easily upgrade to newer kernels.
> >> >> +
> >> >> +    Default value:
> >> >> +      security.selinux, security.SMACK64, security.ima, security.capability
> >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> >> >> index c8fa0aa..4d1c51e 100644
> >> >> --- a/security/integrity/evm/evm.h
> >> >> +++ b/security/integrity/evm/evm.h
> >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
> >> >>
> >> >>  #define EVM_HMAC_ATTR_FSUUID              0x0001
> >> >>
> >> >> +struct evm_hmac_xattr {
> >> >> +  struct list_head list;
> >> >> +  char *xattr;
> >> >> +};
> >> >> +
> >> >>  /* List of EVM protected security xattrs */
> >> >> -extern char *evm_config_xattrnames[];
> >> >> +extern struct list_head evm_hmac_xattrs;
> >> >>
> >> >>  int evm_init_key(void);
> >> >>  int evm_update_evmxattr(struct dentry *dentry,
> >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> >> >> index ab034e5..7e5bfb7 100644
> >> >> --- a/security/integrity/evm/evm_crypto.c
> >> >> +++ b/security/integrity/evm/evm_crypto.c
> >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> >> >>  {
> >> >>    struct inode *inode = dentry->d_inode;
> >> >>    struct shash_desc *desc;
> >> >> -  char **xattrname;
> >> >> +  struct evm_hmac_xattr *entry;
> >> >>    size_t xattr_size = 0;
> >> >>    char *xattr_value = NULL;
> >> >>    int error;
> >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> >> >>            return PTR_ERR(desc);
> >> >>
> >> >>    error = -ENODATA;
> >> >> -  for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> >> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list) {
> >> >>            if ((req_xattr_name && req_xattr_value)
> >> >> -              && !strcmp(*xattrname, req_xattr_name)) {
> >> >> +              && !strcmp(entry->xattr, req_xattr_name)) {
> >> >>                    error = 0;
> >> >>                    crypto_shash_update(desc, (const u8 *)req_xattr_value,
> >> >>                                         req_xattr_value_len);
> >> >>                    continue;
> >> >>            }
> >> >> -          size = vfs_getxattr_alloc(dentry, *xattrname,
> >> >> +          size = vfs_getxattr_alloc(dentry, entry->xattr,
> >> >>                                      &xattr_value, xattr_size, GFP_NOFS);
> >> >>            if (size == -ENOMEM) {
> >> >>                    error = -ENOMEM;
> >> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> >> >> index 9c05929..13e03ad 100644
> >> >> --- a/security/integrity/evm/evm_main.c
> >> >> +++ b/security/integrity/evm/evm_main.c
> >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
> >> >>  char *evm_hash = "sha1";
> >> >>  int evm_hmac_attrs;
> >> >>
> >> >> -char *evm_config_xattrnames[] = {
> >> >> -#ifdef CONFIG_SECURITY_SELINUX
> >> >> -  XATTR_NAME_SELINUX,
> >> >> -#endif
> >> >> -#ifdef CONFIG_SECURITY_SMACK
> >> >> -  XATTR_NAME_SMACK,
> >> >> -#endif
> >> >> -#ifdef CONFIG_IMA_APPRAISE
> >> >> -  XATTR_NAME_IMA,
> >> >> -#endif
> >> >> -  XATTR_NAME_CAPS,
> >> >> -  NULL
> >> >> -};
> >> >> +LIST_HEAD(evm_hmac_xattrs);
> >> >>
> >> >>  static int evm_fixmode;
> >> >>  static int __init evm_set_fixmode(char *str)
> >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
> >> >>  {
> >> >>    char *attrs = CONFIG_EVM_HMAC_ATTRS;
> >> >>    char *p;
> >> >> +  char *xattrs = CONFIG_EVM_HMAC_XATTRS;
> >> >> +  struct evm_hmac_xattr *entry;
> >> >>
> >> >>    while ((p = strsep(&attrs, ", \t"))) {
> >> >>            if (!strcmp(p, "fsuuid"))
> >> >>                    evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
> >> >>    }
> >> >>    pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
> >> >> +  while ((p = strsep(&xattrs, ", \t"))) {
> >> >> +#ifndef CONFIG_SECURITY_SELINUX
> >> >> +          if (!strcmp(p, XATTR_NAME_SELINUX))
> >> >> +                  continue;
> >> >> +#endif
> >> >> +#ifndef CONFIG_SECURITY_SMACK
> >> >> +          if (strstr(p, "SMACK64"))
> >> >> +                  continue;
> >> >> +#endif
> >> > As you mentioned, filesystems previously only included
> >> > 'security.smack64' in the HMAC calculation.  This patch includes all
> >> > xattrs prefixed with smack64.  All previously labeled filesystems would
> >> > need to be relabeled.
> >> >
> >> > Mimi
> >>
> >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC
> >> and SMACK64TRANSMUTE attributes are optional. You do not want these
> >> attributes in most cases. Few files should actually have them. They
> >> should only be used if they are there, and ignored otherwise. If that
> >> is not possible it is better to ignore them completely.
> >
> > Going forward the code would work just fine.  All smack64 prefixed
> > labels would be included in the HMAC calculation.  My concern is for
> > existing labeled filesystems, which only included the SMACK64 label in
> > the HMAC calculation, but have other SMACK64 labels.  The HMAC
> > verification would fail for these files.
> >
> 
> Hello,
> 
> There is no problem for existing labeled filesystems.
> 
> Patch does not work with SMACK64 prefix, but with exact name.
> 
> Only xattrs listed in the configuration options are included
> By default "security.selinux, security.SMACK64, security.ima,
> security.capability"
> which correspond to EVM current functionality...
> 
> Existing systems using new kernel may stick to original EVM xattr set.
> 
> But new systems might prefer to add "more" xattrs..
> 
> This patch makes it flexible...

Ok, the list of xattrs included in the HMAC calculation is build time
configurable and runtime verified.

thanks,

Mimi

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Dmitry Kasatkin]

On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote:
>> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote:
>> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
>> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> >> >> EVM currently uses source hard coded list of xattrs which needs to be
>> >> >> included into the HMAC calculation. This is very unflexible.
>> >> >> Adding new attributes requires modifcation of the source code and
>> >> >> prevents building the kernel which works with previously labeled
>> >> >> filesystems.
>> >> >>
>> >> >> Early versions of Smack used only one xattr security.SMACK64,
>> >> >> which is protected by EVM. Now Smack has a few more attributes and
>> >> >> they are not protected. Adding support to the source code makes it
>> >> >> impossible to use new kernel with previousely labeled filesystems.
>> >> > I think this patch will break any existing filesystems labeled with
>> >> > 'security.smack64'.  Details inline.
>> >> >
>> >> >> This patch replaces hardcoded xattr array with dynamic list which is
>> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
>> >> >> kernel with with support of older and newer EVM HMAC formats.
>
> So instead of having a single kernel, this allows you to build different
> kernels with different xattr labels included in the HMAC.  Wouldn't you
> want a migration mode, similar to 'fix' mode, that only updates the
> HMAC, if the existing HMAC verified based on the prior set of xattrs?
>

It would require to maintain 2 sets of xattrs: "old" and "new".
What if "new" becomes "old" again, while there were still not updated
previous "old" :)

I think it would bring unnecessary complexity.
System designers define what xattrs they want to protect and stick with that.
Filesystems stays anyway, but allows to upgrade to new kernels.

If someone really wants to upgrade the system, just use "evm=fix" and run
recursive fix.., e.g. 'evmctl -r ima_fix /'

- Dmitry

>> >> >>
>> >> >> Possible future extension will be to read xattr list from the kernel
>> >> >> command line or from securityfs entry.
>> >> >>
>> >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> >> >> ---
>> >> >>  security/integrity/evm/Kconfig      | 10 ++++++
>> >> >>  security/integrity/evm/evm.h        |  7 +++-
>> >> >>  security/integrity/evm/evm_crypto.c |  8 ++---
>> >> >>  security/integrity/evm/evm_main.c   | 69 +++++++++++++++++++------------------
>> >> >>  4 files changed, 55 insertions(+), 39 deletions(-)
>> >> >>
>> >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
>> >> >> index 2be51fa..06237b8 100644
>> >> >> --- a/security/integrity/evm/Kconfig
>> >> >> +++ b/security/integrity/evm/Kconfig
>> >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS
>> >> >>      WARNING: changing the HMAC calculation method or adding
>> >> >>      additional info to the calculation, requires existing EVM
>> >> >>      labeled file systems to be relabeled.
>> >> >> +
>> >> >> +config EVM_HMAC_XATTRS
>> >> >> +  string "HMAC xattrs"
>> >> >> +  default "security.selinux security.SMACK64 security.ima security.capability"
>> >> >> +  help
>> >> >> +    This options allows to specify list of extended attributes included into HMAC
>> >> >> +    calculation. It makes it possible easily upgrade to newer kernels.
>> >> >> +
>> >> >> +    Default value:
>> >> >> +      security.selinux, security.SMACK64, security.ima, security.capability
>> >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> >> >> index c8fa0aa..4d1c51e 100644
>> >> >> --- a/security/integrity/evm/evm.h
>> >> >> +++ b/security/integrity/evm/evm.h
>> >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm;
>> >> >>
>> >> >>  #define EVM_HMAC_ATTR_FSUUID              0x0001
>> >> >>
>> >> >> +struct evm_hmac_xattr {
>> >> >> +  struct list_head list;
>> >> >> +  char *xattr;
>> >> >> +};
>> >> >> +
>> >> >>  /* List of EVM protected security xattrs */
>> >> >> -extern char *evm_config_xattrnames[];
>> >> >> +extern struct list_head evm_hmac_xattrs;
>> >> >>
>> >> >>  int evm_init_key(void);
>> >> >>  int evm_update_evmxattr(struct dentry *dentry,
>> >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> >> >> index ab034e5..7e5bfb7 100644
>> >> >> --- a/security/integrity/evm/evm_crypto.c
>> >> >> +++ b/security/integrity/evm/evm_crypto.c
>> >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>> >> >>  {
>> >> >>    struct inode *inode = dentry->d_inode;
>> >> >>    struct shash_desc *desc;
>> >> >> -  char **xattrname;
>> >> >> +  struct evm_hmac_xattr *entry;
>> >> >>    size_t xattr_size = 0;
>> >> >>    char *xattr_value = NULL;
>> >> >>    int error;
>> >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>> >> >>            return PTR_ERR(desc);
>> >> >>
>> >> >>    error = -ENODATA;
>> >> >> -  for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>> >> >> +  list_for_each_entry(entry, &evm_hmac_xattrs, list) {
>> >> >>            if ((req_xattr_name && req_xattr_value)
>> >> >> -              && !strcmp(*xattrname, req_xattr_name)) {
>> >> >> +              && !strcmp(entry->xattr, req_xattr_name)) {
>> >> >>                    error = 0;
>> >> >>                    crypto_shash_update(desc, (const u8 *)req_xattr_value,
>> >> >>                                         req_xattr_value_len);
>> >> >>                    continue;
>> >> >>            }
>> >> >> -          size = vfs_getxattr_alloc(dentry, *xattrname,
>> >> >> +          size = vfs_getxattr_alloc(dentry, entry->xattr,
>> >> >>                                      &xattr_value, xattr_size, GFP_NOFS);
>> >> >>            if (size == -ENOMEM) {
>> >> >>                    error = -ENOMEM;
>> >> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> >> >> index 9c05929..13e03ad 100644
>> >> >> --- a/security/integrity/evm/evm_main.c
>> >> >> +++ b/security/integrity/evm/evm_main.c
>> >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)";
>> >> >>  char *evm_hash = "sha1";
>> >> >>  int evm_hmac_attrs;
>> >> >>
>> >> >> -char *evm_config_xattrnames[] = {
>> >> >> -#ifdef CONFIG_SECURITY_SELINUX
>> >> >> -  XATTR_NAME_SELINUX,
>> >> >> -#endif
>> >> >> -#ifdef CONFIG_SECURITY_SMACK
>> >> >> -  XATTR_NAME_SMACK,
>> >> >> -#endif
>> >> >> -#ifdef CONFIG_IMA_APPRAISE
>> >> >> -  XATTR_NAME_IMA,
>> >> >> -#endif
>> >> >> -  XATTR_NAME_CAPS,
>> >> >> -  NULL
>> >> >> -};
>> >> >> +LIST_HEAD(evm_hmac_xattrs);
>> >> >>
>> >> >>  static int evm_fixmode;
>> >> >>  static int __init evm_set_fixmode(char *str)
>> >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void)
>> >> >>  {
>> >> >>    char *attrs = CONFIG_EVM_HMAC_ATTRS;
>> >> >>    char *p;
>> >> >> +  char *xattrs = CONFIG_EVM_HMAC_XATTRS;
>> >> >> +  struct evm_hmac_xattr *entry;
>> >> >>
>> >> >>    while ((p = strsep(&attrs, ", \t"))) {
>> >> >>            if (!strcmp(p, "fsuuid"))
>> >> >>                    evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID;
>> >> >>    }
>> >> >>    pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
>> >> >> +  while ((p = strsep(&xattrs, ", \t"))) {
>> >> >> +#ifndef CONFIG_SECURITY_SELINUX
>> >> >> +          if (!strcmp(p, XATTR_NAME_SELINUX))
>> >> >> +                  continue;
>> >> >> +#endif
>> >> >> +#ifndef CONFIG_SECURITY_SMACK
>> >> >> +          if (strstr(p, "SMACK64"))
>> >> >> +                  continue;
>> >> >> +#endif
>> >> > As you mentioned, filesystems previously only included
>> >> > 'security.smack64' in the HMAC calculation.  This patch includes all
>> >> > xattrs prefixed with smack64.  All previously labeled filesystems would
>> >> > need to be relabeled.
>> >> >
>> >> > Mimi
>> >>
>> >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC
>> >> and SMACK64TRANSMUTE attributes are optional. You do not want these
>> >> attributes in most cases. Few files should actually have them. They
>> >> should only be used if they are there, and ignored otherwise. If that
>> >> is not possible it is better to ignore them completely.
>> >
>> > Going forward the code would work just fine.  All smack64 prefixed
>> > labels would be included in the HMAC calculation.  My concern is for
>> > existing labeled filesystems, which only included the SMACK64 label in
>> > the HMAC calculation, but have other SMACK64 labels.  The HMAC
>> > verification would fail for these files.
>> >
>>
>> Hello,
>>
>> There is no problem for existing labeled filesystems.
>>
>> Patch does not work with SMACK64 prefix, but with exact name.
>>
>> Only xattrs listed in the configuration options are included
>> By default "security.selinux, security.SMACK64, security.ima,
>> security.capability"
>> which correspond to EVM current functionality...
>>
>> Existing systems using new kernel may stick to original EVM xattr set.
>>
>> But new systems might prefer to add "more" xattrs..
>>
>> This patch makes it flexible...
>
> Ok, the list of xattrs included in the HMAC calculation is build time
> configurable and runtime verified.
>

yes.

> thanks,
>
> Mimi
>
>



-- 
Thanks,
Dmitry

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Mimi Zohar]

On Wed, 2014-03-05 at 11:26 +0200, Dmitry Kasatkin wrote: 
> On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote:
> >> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote:
> >> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
> >> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
> >> >> >> EVM currently uses source hard coded list of xattrs which needs to be
> >> >> >> included into the HMAC calculation. This is very unflexible.
> >> >> >> Adding new attributes requires modifcation of the source code and
> >> >> >> prevents building the kernel which works with previously labeled
> >> >> >> filesystems.
> >> >> >>
> >> >> >> Early versions of Smack used only one xattr security.SMACK64,
> >> >> >> which is protected by EVM. Now Smack has a few more attributes and
> >> >> >> they are not protected. Adding support to the source code makes it
> >> >> >> impossible to use new kernel with previousely labeled filesystems.
> >> >> > I think this patch will break any existing filesystems labeled with
> >> >> > 'security.smack64'.  Details inline.
> >> >> >
> >> >> >> This patch replaces hardcoded xattr array with dynamic list which is
> >> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
> >> >> >> kernel with with support of older and newer EVM HMAC formats.
> >
> > So instead of having a single kernel, this allows you to build different
> > kernels with different xattr labels included in the HMAC.  Wouldn't you
> > want a migration mode, similar to 'fix' mode, that only updates the
> > HMAC, if the existing HMAC verified based on the prior set of xattrs?
> >
> 
> It would require to maintain 2 sets of xattrs: "old" and "new".
> What if "new" becomes "old" again, while there were still not updated
> previous "old" :)
> 
> I think it would bring unnecessary complexity.
> System designers define what xattrs they want to protect and stick with that.
> Filesystems stays anyway, but allows to upgrade to new kernels.
> 
> If someone really wants to upgrade the system, just use "evm=fix" and run
> recursive fix.., e.g. 'evmctl -r ima_fix /'

Right, but instead of fixing everything, we would want a "safe" fix.
Verify the existing HMAC, before updating the HMAC based on the new set
of xattrs.  With this design, only an "old" and "new" set of xattrs
would be needed.

Mimi

Re: [PATCH 8/8] evm: introduce EVM hmac xattr list

[Dmitry Kasatkin]

On Wed, Mar 5, 2014 at 6:04 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2014-03-05 at 11:26 +0200, Dmitry Kasatkin wrote:
>> On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote:
>> >> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> >> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote:
>> >> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote:
>> >> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote:
>> >> >> >> EVM currently uses source hard coded list of xattrs which needs to be
>> >> >> >> included into the HMAC calculation. This is very unflexible.
>> >> >> >> Adding new attributes requires modifcation of the source code and
>> >> >> >> prevents building the kernel which works with previously labeled
>> >> >> >> filesystems.
>> >> >> >>
>> >> >> >> Early versions of Smack used only one xattr security.SMACK64,
>> >> >> >> which is protected by EVM. Now Smack has a few more attributes and
>> >> >> >> they are not protected. Adding support to the source code makes it
>> >> >> >> impossible to use new kernel with previousely labeled filesystems.
>> >> >> > I think this patch will break any existing filesystems labeled with
>> >> >> > 'security.smack64'.  Details inline.
>> >> >> >
>> >> >> >> This patch replaces hardcoded xattr array with dynamic list which is
>> >> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build
>> >> >> >> kernel with with support of older and newer EVM HMAC formats.
>> >
>> > So instead of having a single kernel, this allows you to build different
>> > kernels with different xattr labels included in the HMAC.  Wouldn't you
>> > want a migration mode, similar to 'fix' mode, that only updates the
>> > HMAC, if the existing HMAC verified based on the prior set of xattrs?
>> >
>>
>> It would require to maintain 2 sets of xattrs: "old" and "new".
>> What if "new" becomes "old" again, while there were still not updated
>> previous "old" :)
>>
>> I think it would bring unnecessary complexity.
>> System designers define what xattrs they want to protect and stick with that.
>> Filesystems stays anyway, but allows to upgrade to new kernels.
>>
>> If someone really wants to upgrade the system, just use "evm=fix" and run
>> recursive fix.., e.g. 'evmctl -r ima_fix /'
>
> Right, but instead of fixing everything, we would want a "safe" fix.
> Verify the existing HMAC, before updating the HMAC based on the new set
> of xattrs.  With this design, only an "old" and "new" set of xattrs
> would be needed.
>

This can be the next step further.
Now we need to support new xattrs in the kernel, without breaking
existing labeled filesystem.

Dmitry


> Mimi
>



-- 
Thanks,
Dmitry