Skip to content

Commit 642b151

Browse files
committed
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()
2 parents 4508896 + 8433856 commit 642b151

File tree

5 files changed

+40
-34
lines changed

5 files changed

+40
-34
lines changed

security/integrity/evm/evm_crypto.c

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
7373
{
7474
long rc;
7575
const char *algo;
76-
struct crypto_shash **tfm;
76+
struct crypto_shash **tfm, *tmp_tfm;
7777
struct shash_desc *desc;
7878

7979
if (type == EVM_XATTR_HMAC) {
@@ -91,31 +91,31 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
9191
algo = hash_algo_name[hash_algo];
9292
}
9393

94-
if (*tfm == NULL) {
95-
mutex_lock(&mutex);
96-
if (*tfm)
97-
goto out;
98-
*tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
99-
if (IS_ERR(*tfm)) {
100-
rc = PTR_ERR(*tfm);
101-
pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
102-
*tfm = NULL;
94+
if (*tfm)
95+
goto alloc;
96+
mutex_lock(&mutex);
97+
if (*tfm)
98+
goto unlock;
99+
100+
tmp_tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
101+
if (IS_ERR(tmp_tfm)) {
102+
pr_err("Can not allocate %s (reason: %ld)\n", algo,
103+
PTR_ERR(tmp_tfm));
104+
mutex_unlock(&mutex);
105+
return ERR_CAST(tmp_tfm);
106+
}
107+
if (type == EVM_XATTR_HMAC) {
108+
rc = crypto_shash_setkey(tmp_tfm, evmkey, evmkey_len);
109+
if (rc) {
110+
crypto_free_shash(tmp_tfm);
103111
mutex_unlock(&mutex);
104112
return ERR_PTR(rc);
105113
}
106-
if (type == EVM_XATTR_HMAC) {
107-
rc = crypto_shash_setkey(*tfm, evmkey, evmkey_len);
108-
if (rc) {
109-
crypto_free_shash(*tfm);
110-
*tfm = NULL;
111-
mutex_unlock(&mutex);
112-
return ERR_PTR(rc);
113-
}
114-
}
115-
out:
116-
mutex_unlock(&mutex);
117114
}
118-
115+
*tfm = tmp_tfm;
116+
unlock:
117+
mutex_unlock(&mutex);
118+
alloc:
119119
desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm),
120120
GFP_KERNEL);
121121
if (!desc)
@@ -207,7 +207,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
207207
data->hdr.length = crypto_shash_digestsize(desc->tfm);
208208

209209
error = -ENODATA;
210-
list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
210+
list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
211211
bool is_ima = false;
212212

213213
if (strcmp(xattr->name, XATTR_NAME_IMA) == 0)

security/integrity/evm/evm_main.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
9797
if (!(inode->i_opflags & IOP_XATTR))
9898
return -EOPNOTSUPP;
9999

100-
list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
100+
list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
101101
error = __vfs_getxattr(dentry, inode, xattr->name, NULL, 0);
102102
if (error < 0) {
103103
if (error == -ENODATA)
@@ -228,7 +228,7 @@ static int evm_protected_xattr(const char *req_xattr_name)
228228
struct xattr_list *xattr;
229229

230230
namelen = strlen(req_xattr_name);
231-
list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) {
231+
list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
232232
if ((strlen(xattr->name) == namelen)
233233
&& (strncmp(req_xattr_name, xattr->name, namelen) == 0)) {
234234
found = 1;

security/integrity/evm/evm_secfs.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,14 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
232232
goto out;
233233
}
234234

235-
/* Guard against races in evm_read_xattrs */
235+
/*
236+
* xattr_list_mutex guards against races in evm_read_xattrs().
237+
* Entries are only added to the evm_config_xattrnames list
238+
* and never deleted. Therefore, the list is traversed
239+
* using list_for_each_entry_lockless() without holding
240+
* the mutex in evm_calc_hmac_or_hash(), evm_find_protected_xattrs()
241+
* and evm_protected_xattr().
242+
*/
236243
mutex_lock(&xattr_list_mutex);
237244
list_for_each_entry(tmp, &evm_config_xattrnames, list) {
238245
if (strcmp(xattr->name, tmp->name) == 0) {

security/integrity/ima/ima_crypto.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
411411
loff_t i_size;
412412
int rc;
413413
struct file *f = file;
414-
bool new_file_instance = false, modified_flags = false;
414+
bool new_file_instance = false, modified_mode = false;
415415

416416
/*
417417
* 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)
431431
f = dentry_open(&file->f_path, flags, file->f_cred);
432432
if (IS_ERR(f)) {
433433
/*
434-
* Cannot open the file again, lets modify f_flags
434+
* Cannot open the file again, lets modify f_mode
435435
* of original and continue
436436
*/
437437
pr_info_ratelimited("Unable to reopen file for reading.\n");
438438
f = file;
439-
f->f_flags |= FMODE_READ;
440-
modified_flags = true;
439+
f->f_mode |= FMODE_READ;
440+
modified_mode = true;
441441
} else {
442442
new_file_instance = true;
443443
}
@@ -455,8 +455,8 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
455455
out:
456456
if (new_file_instance)
457457
fput(f);
458-
else if (modified_flags)
459-
f->f_flags &= ~FMODE_READ;
458+
else if (modified_mode)
459+
f->f_mode &= ~FMODE_READ;
460460
return rc;
461461
}
462462

security/integrity/ima/ima_fs.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
338338
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
339339
"policy_update", "signed policy required",
340340
1, 0);
341-
if (ima_appraise & IMA_APPRAISE_ENFORCE)
342-
result = -EACCES;
341+
result = -EACCES;
343342
} else {
344343
result = ima_parse_add_rule(data);
345344
}

0 commit comments

Comments
 (0)