diff options
| author | Linus Torvalds <torvalds@linux-foundation.org> | 2020-05-18 11:29:21 -0700 | 
|---|---|---|
| committer | Linus Torvalds <torvalds@linux-foundation.org> | 2020-05-18 11:29:21 -0700 | 
| commit | 642b151f45dd54809ea00ecd3976a56c1ec9b53d (patch) | |
| tree | caf4db13f90f10082701e3f0d38379701e4e5a35 | |
| parent | 45088963ca9cdc3df50dd7b1b63e1dc9dcc6375e (diff) | |
| parent | 8433856947217ebb5697a8ff9c4c9cad4639a2cf (diff) | |
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()
| -rw-r--r-- | security/integrity/evm/evm_crypto.c | 46 | ||||
| -rw-r--r-- | security/integrity/evm/evm_main.c | 4 | ||||
| -rw-r--r-- | security/integrity/evm/evm_secfs.c | 9 | ||||
| -rw-r--r-- | security/integrity/ima/ima_crypto.c | 12 | ||||
| -rw-r--r-- | security/integrity/ima/ima_fs.c | 3 | 
5 files changed, 40 insertions, 34 deletions
| diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 35682852ddea..764b896cd628 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -73,7 +73,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)  {  	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 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)  		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 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,  	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 d361d7fdafc4..0d36259b690d 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -97,7 +97,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry)  	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 @@ static int evm_protected_xattr(const char *req_xattr_name)  	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 39ad1038d45d..cfc3075769bb 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -232,7 +232,14 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,  		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 423c84f95a14..88b5e288f241 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -411,7 +411,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)  	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 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)  		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 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)  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 a71e822a6e92..3efc8308ad26 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -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,  				    "policy_update", "signed policy required",  				    1, 0); -		if (ima_appraise & IMA_APPRAISE_ENFORCE) -			result = -EACCES; +		result = -EACCES;  	} else {  		result = ima_parse_add_rule(data);  	} | 
