Merge branch 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
Pull integrity fixes from Mimi Zohar:
"A couple of miscellaneous bug fixes for the integrity subsystem:
IMA:
- Properly modify the open flags in order to calculate the file hash.
- On systems requiring the IMA policy to be signed, the policy is
loaded differently. Don't differentiate between "enforce" and
either "log" or "fix" modes how the policy is loaded.
EVM:
- Two patches to fix an EVM race condition, normally the result of
attempting to load an unsupported hash algorithm.
- Use the lockless RCU version for walking an append only list"
* 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity:
evm: Fix a small race in init_desc()
evm: Fix RCU list related warnings
ima: Fix return value of ima_write_policy()
evm: Check also if *tfm is an error pointer in init_desc()
ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash()
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 3568285..764b896 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -73,7 +73,7 @@
{
long rc;
const char *algo;
- struct crypto_shash **tfm;
+ struct crypto_shash **tfm, *tmp_tfm;
struct shash_desc *desc;
if (type == EVM_XATTR_HMAC) {
@@ -91,31 +91,31 @@
algo = hash_algo_name[hash_algo];
}
- if (*tfm == NULL) {
- mutex_lock(&mutex);
- if (*tfm)
- goto out;
- *tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
- if (IS_ERR(*tfm)) {
- rc = PTR_ERR(*tfm);
- pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
- *tfm = NULL;
+ if (*tfm)
+ goto alloc;
+ mutex_lock(&mutex);
+ if (*tfm)
+ goto unlock;
+
+ tmp_tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
+ if (IS_ERR(tmp_tfm)) {
+ pr_err("Can not allocate %s (reason: %ld)\n", algo,
+ PTR_ERR(tmp_tfm));
+ mutex_unlock(&mutex);
+ return ERR_CAST(tmp_tfm);
+ }
+ if (type == EVM_XATTR_HMAC) {
+ rc = crypto_shash_setkey(tmp_tfm, evmkey, evmkey_len);
+ if (rc) {
+ crypto_free_shash(tmp_tfm);
mutex_unlock(&mutex);
return ERR_PTR(rc);
}
- if (type == EVM_XATTR_HMAC) {
- rc = crypto_shash_setkey(*tfm, evmkey, evmkey_len);
- if (rc) {
- crypto_free_shash(*tfm);
- *tfm = NULL;
- mutex_unlock(&mutex);
- return ERR_PTR(rc);
- }
- }
-out:
- mutex_unlock(&mutex);
}
-
+ *tfm = tmp_tfm;
+unlock:
+ mutex_unlock(&mutex);
+alloc:
desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm),
GFP_KERNEL);
if (!desc)
@@ -207,7 +207,7 @@
data->hdr.length = crypto_shash_digestsize(desc->tfm);
error = -ENODATA;
- list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
+ list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
bool is_ima = false;
if (strcmp(xattr->name, XATTR_NAME_IMA) == 0)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index d361d7f..0d36259 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -97,7 +97,7 @@
if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP;
- list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
+ list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
error = __vfs_getxattr(dentry, inode, xattr->name, NULL, 0);
if (error < 0) {
if (error == -ENODATA)
@@ -228,7 +228,7 @@
struct xattr_list *xattr;
namelen = strlen(req_xattr_name);
- list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
+ list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
if ((strlen(xattr->name) == namelen)
&& (strncmp(req_xattr_name, xattr->name, namelen) == 0)) {
found = 1;
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 39ad103..cfc3075 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -232,7 +232,14 @@
goto out;
}
- /* Guard against races in evm_read_xattrs */
+ /*
+ * xattr_list_mutex guards against races in evm_read_xattrs().
+ * Entries are only added to the evm_config_xattrnames list
+ * and never deleted. Therefore, the list is traversed
+ * using list_for_each_entry_lockless() without holding
+ * the mutex in evm_calc_hmac_or_hash(), evm_find_protected_xattrs()
+ * and evm_protected_xattr().
+ */
mutex_lock(&xattr_list_mutex);
list_for_each_entry(tmp, &evm_config_xattrnames, list) {
if (strcmp(xattr->name, tmp->name) == 0) {
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 423c84f..88b5e28 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -411,7 +411,7 @@
loff_t i_size;
int rc;
struct file *f = file;
- bool new_file_instance = false, modified_flags = false;
+ bool new_file_instance = false, modified_mode = false;
/*
* For consistency, fail file's opened with the O_DIRECT flag on
@@ -431,13 +431,13 @@
f = dentry_open(&file->f_path, flags, file->f_cred);
if (IS_ERR(f)) {
/*
- * Cannot open the file again, lets modify f_flags
+ * Cannot open the file again, lets modify f_mode
* of original and continue
*/
pr_info_ratelimited("Unable to reopen file for reading.\n");
f = file;
- f->f_flags |= FMODE_READ;
- modified_flags = true;
+ f->f_mode |= FMODE_READ;
+ modified_mode = true;
} else {
new_file_instance = true;
}
@@ -455,8 +455,8 @@
out:
if (new_file_instance)
fput(f);
- else if (modified_flags)
- f->f_flags &= ~FMODE_READ;
+ else if (modified_mode)
+ f->f_mode &= ~FMODE_READ;
return rc;
}
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index a71e822..3efc830 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -338,8 +338,7 @@
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
"policy_update", "signed policy required",
1, 0);
- if (ima_appraise & IMA_APPRAISE_ENFORCE)
- result = -EACCES;
+ result = -EACCES;
} else {
result = ima_parse_add_rule(data);
}
