Skip to content

Commit 8433856

Browse files
Dan Carpentermimizohar
authored andcommitted
evm: Fix a small race in init_desc()
The IS_ERR_OR_NULL() function has two conditions and if we got really unlucky we could hit a race where "ptr" started as an error pointer and then was set to NULL. Both conditions would be false even though the pointer at the end was NULL. This patch fixes the problem by ensuring that "*tfm" can only be NULL or valid. I have introduced a "tmp_tfm" variable to make that work. I also reversed a condition and pulled the code in one tab. Reported-by: Roberto Sassu <[email protected]> Fixes: 53de3b0 ("evm: Check also if *tfm is an error pointer in init_desc()") Signed-off-by: Dan Carpenter <[email protected]> Acked-by: Roberto Sassu <[email protected]> Acked-by: Krzysztof Struczynski <[email protected]> Signed-off-by: Mimi Zohar <[email protected]>
1 parent 770f605 commit 8433856

File tree

1 file changed

+22
-22
lines changed

1 file changed

+22
-22
lines changed

security/integrity/evm/evm_crypto.c

Lines changed: 22 additions & 22 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 (IS_ERR_OR_NULL(*tfm)) {
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)

0 commit comments

Comments
 (0)