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()
This commit is contained in:
Linus Torvalds 2020-05-18 11:29:21 -07:00
commit 642b151f45
5 changed files with 40 additions and 34 deletions

View file

@ -73,7 +73,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
{ {
long rc; long rc;
const char *algo; const char *algo;
struct crypto_shash **tfm; struct crypto_shash **tfm, *tmp_tfm;
struct shash_desc *desc; struct shash_desc *desc;
if (type == EVM_XATTR_HMAC) { if (type == EVM_XATTR_HMAC) {
@ -91,31 +91,31 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
algo = hash_algo_name[hash_algo]; algo = hash_algo_name[hash_algo];
} }
if (*tfm == NULL) { if (*tfm)
mutex_lock(&mutex); goto alloc;
if (*tfm) mutex_lock(&mutex);
goto out; if (*tfm)
*tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD); goto unlock;
if (IS_ERR(*tfm)) {
rc = PTR_ERR(*tfm); tmp_tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
pr_err("Can not allocate %s (reason: %ld)\n", algo, rc); if (IS_ERR(tmp_tfm)) {
*tfm = NULL; 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); mutex_unlock(&mutex);
return ERR_PTR(rc); 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), desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm),
GFP_KERNEL); GFP_KERNEL);
if (!desc) if (!desc)
@ -207,7 +207,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
data->hdr.length = crypto_shash_digestsize(desc->tfm); data->hdr.length = crypto_shash_digestsize(desc->tfm);
error = -ENODATA; 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; bool is_ima = false;
if (strcmp(xattr->name, XATTR_NAME_IMA) == 0) if (strcmp(xattr->name, XATTR_NAME_IMA) == 0)

View file

@ -97,7 +97,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
if (!(inode->i_opflags & IOP_XATTR)) if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP; 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); error = __vfs_getxattr(dentry, inode, xattr->name, NULL, 0);
if (error < 0) { if (error < 0) {
if (error == -ENODATA) if (error == -ENODATA)
@ -228,7 +228,7 @@ static int evm_protected_xattr(const char *req_xattr_name)
struct xattr_list *xattr; struct xattr_list *xattr;
namelen = strlen(req_xattr_name); 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) if ((strlen(xattr->name) == namelen)
&& (strncmp(req_xattr_name, xattr->name, namelen) == 0)) { && (strncmp(req_xattr_name, xattr->name, namelen) == 0)) {
found = 1; found = 1;

View file

@ -232,7 +232,14 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
goto out; 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); mutex_lock(&xattr_list_mutex);
list_for_each_entry(tmp, &evm_config_xattrnames, list) { list_for_each_entry(tmp, &evm_config_xattrnames, list) {
if (strcmp(xattr->name, tmp->name) == 0) { if (strcmp(xattr->name, tmp->name) == 0) {

View file

@ -411,7 +411,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
loff_t i_size; loff_t i_size;
int rc; int rc;
struct file *f = file; 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 * For consistency, fail file's opened with the O_DIRECT flag on
@ -431,13 +431,13 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
f = dentry_open(&file->f_path, flags, file->f_cred); f = dentry_open(&file->f_path, flags, file->f_cred);
if (IS_ERR(f)) { 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 * of original and continue
*/ */
pr_info_ratelimited("Unable to reopen file for reading.\n"); pr_info_ratelimited("Unable to reopen file for reading.\n");
f = file; f = file;
f->f_flags |= FMODE_READ; f->f_mode |= FMODE_READ;
modified_flags = true; modified_mode = true;
} else { } else {
new_file_instance = true; new_file_instance = true;
} }
@ -455,8 +455,8 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
out: out:
if (new_file_instance) if (new_file_instance)
fput(f); fput(f);
else if (modified_flags) else if (modified_mode)
f->f_flags &= ~FMODE_READ; f->f_mode &= ~FMODE_READ;
return rc; return rc;
} }

View file

@ -338,8 +338,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
"policy_update", "signed policy required", "policy_update", "signed policy required",
1, 0); 1, 0);
if (ima_appraise & IMA_APPRAISE_ENFORCE) result = -EACCES;
result = -EACCES;
} else { } else {
result = ima_parse_add_rule(data); result = ima_parse_add_rule(data);
} }