diff mbox

[1/2] evm: Don't deadlock if a crypto algorithm is unavailable

Message ID 20180601230244.138560-1-mjg59@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Garrett June 1, 2018, 11:02 p.m. UTC
Trying to instantiate a non-existent crypto algorithm will cause the
kernel to trigger a module load. If EVM appraisal is enabled, this will
in turn trigger appraisal of the module, which will fail because the
crypto algorithm isn't available. Add a CRYPTO_NOLOAD flag and skip
module loading if it's set, and add that flag in the EVM case.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 crypto/api.c                        | 2 +-
 include/linux/crypto.h              | 5 +++++
 security/integrity/evm/evm_crypto.c | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

Comments

Herbert Xu June 2, 2018, 3:54 p.m. UTC | #1
On Fri, Jun 01, 2018 at 04:02:43PM -0700, Matthew Garrett wrote:
> Trying to instantiate a non-existent crypto algorithm will cause the
> kernel to trigger a module load. If EVM appraisal is enabled, this will
> in turn trigger appraisal of the module, which will fail because the
> crypto algorithm isn't available. Add a CRYPTO_NOLOAD flag and skip
> module loading if it's set, and add that flag in the EVM case.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

I don't get it.  Without your patch it will fail because the
EVM appraisal fails.  With your patch it will fail because there
is no algorithm registered.  So what's the difference?

Cheers,
Matthew Garrett June 4, 2018, 8:09 p.m. UTC | #2
On Sat, Jun 2, 2018 at 8:54 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Fri, Jun 01, 2018 at 04:02:43PM -0700, Matthew Garrett wrote:
> > Trying to instantiate a non-existent crypto algorithm will cause the
> > kernel to trigger a module load. If EVM appraisal is enabled, this will
> > in turn trigger appraisal of the module, which will fail because the
> > crypto algorithm isn't available. Add a CRYPTO_NOLOAD flag and skip
> > module loading if it's set, and add that flag in the EVM case.
> >
> > Signed-off-by: Matthew Garrett <mjg59@google.com>
>
> I don't get it.  Without your patch it will fail because the
> EVM appraisal fails.  With your patch it will fail because there
> is no algorithm registered.  So what's the difference?

Without my patch it will deadlock as it recursively calls back into
EVM to perform the module appraisal. Sorry, the description was
unclear on that point.
diff mbox

Patch

diff --git a/crypto/api.c b/crypto/api.c
index 0ee632bba064..7aca9f86c5f3 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -229,7 +229,7 @@  static struct crypto_alg *crypto_larval_lookup(const char *name, u32 type,
 	mask &= ~(CRYPTO_ALG_LARVAL | CRYPTO_ALG_DEAD);
 
 	alg = crypto_alg_lookup(name, type, mask);
-	if (!alg) {
+	if (!alg && !(mask & CRYPTO_NOLOAD)) {
 		request_module("crypto-%s", name);
 
 		if (!((type ^ CRYPTO_ALG_NEED_FALLBACK) & mask &
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6eb06101089f..e8839d3a7559 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -112,6 +112,11 @@ 
  */
 #define CRYPTO_ALG_OPTIONAL_KEY		0x00004000
 
+/*
+ * Don't trigger module loading
+ */
+#define CRYPTO_NOLOAD			0x00008000
+
 /*
  * Transform masks and values (for crt_flags).
  */
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a46fba322340..ccafc9468611 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -97,7 +97,8 @@  static struct shash_desc *init_desc(char type)
 		mutex_lock(&mutex);
 		if (*tfm)
 			goto out;
-		*tfm = crypto_alloc_shash(algo, 0, CRYPTO_ALG_ASYNC);
+		*tfm = crypto_alloc_shash(algo, 0,
+					  CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD);
 		if (IS_ERR(*tfm)) {
 			rc = PTR_ERR(*tfm);
 			pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);