HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
-----Original Message----- From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Wednesday, April 22, 2020 3:45 PM To: Roberto Sassu roberto.sassu@huawei.com Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org; Krzysztof Struczynski krzysztof.struczynski@huawei.com; Silviu Vlasceanu Silviu.Vlasceanu@huawei.com; stable@vger.kernel.org Subject: Re: [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc()
Hi Roberto, Krzysztof,
On Wed, 2020-03-25 at 17:11 +0100, Roberto Sassu wrote:
The mutex in init_desc(), introduced by commit 97426f985729 ("evm:
prevent
racing during tfm allocation") prevents two tasks to concurrently set *tfm. However, checking if *tfm is NULL is not enough, as crypto_alloc_shash() can return an error pointer. The following sequence can happen:
Task A: *tfm = crypto_alloc_shash() <= error pointer Task B: if (*tfm == NULL) <= *tfm is not NULL, use it Task B: rc = crypto_shash_init(desc) <= panic Task A: *tfm = NULL
This patch uses the IS_ERR_OR_NULL macro to determine whether or not
a new
crypto context must be created.
Cc: stable@vger.kernel.org Fixes: 97426f985729 ("evm: prevent racing during tfm allocation")
Thank you. True, this commit introduced the mutex, but the actual problem is most likely the result of a crypto algorithm not being configured. Depending on the kernel and which crypto algorithms are enabled, verifying an EVM signature might not be possible. In the embedded environment, where the entire filesystem is updated, there shouldn't be any unknown EVM signature algorithms.
Hi Mimi
right, the actual commit that introduced the issue is:
d46eb3699502b ("evm: crypto hash replaced by shash")
In case Greg or Sasha decide this patch should be backported, including the context/motivation in the patch description (first paragraph) would be helpful.
Ok. The main motivation is to avoid kernel panic, especially if there are many files that require an unsupported hash algorithm, as it would increase the likelihood of the race condition I described.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
Co-developed-by: Krzysztof Struczynski
krzysztof.struczynski@huawei.com
Signed-off-by: Krzysztof Struczynski krzysztof.struczynski@huawei.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
security/integrity/evm/evm_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_crypto.c
b/security/integrity/evm/evm_crypto.c
index 35682852ddea..77ad1e5a93e4 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -91,7 +91,7 @@ static struct shash_desc *init_desc(char type, uint8_t
hash_algo)
algo = hash_algo_name[hash_algo];}
- if (*tfm == NULL) {
- if (IS_ERR_OR_NULL(*tfm)) { mutex_lock(&mutex); if (*tfm) goto out;