diff mbox

crypto: hang in crypto_larval_lookup

Message ID 20170225151707.GA28667@gondor.apana.org.au (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show

Commit Message

Herbert Xu Feb. 25, 2017, 3:17 p.m. UTC
On Fri, Feb 24, 2017 at 08:44:00PM -0300, Marcelo Cerri wrote:
>
> This is probably caused by the way that the xts template is handling the
> underline algorithm selection.

Good catch.  I think the bigger issue here is that when requesting
for an XTS that doesn't need a fallback we shouldn't be using an
underlying ECB that needs one.  So this patch should fix the hang.

Thanks,

---8<---
Subject: crypto: xts - Propagate NEED_FALLBACK bit

When we're used as a fallback algorithm, we should propagate
the NEED_FALLBACK bit when searching for the underlying ECB mode.

This just happens to fix a hang too because otherwise the search
may end up loading the same module that triggered this XTS creation.

Fixes: f1c131b45410 ("crypto: xts - Convert to skcipher")
Reported-by: Harald Freudenberger <freude@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

Marcelo Henrique Cerri Feb. 25, 2017, 7:20 p.m. UTC | #1
On Sat, Feb 25, 2017 at 11:17:07PM +0800, Herbert Xu wrote:
> On Fri, Feb 24, 2017 at 08:44:00PM -0300, Marcelo Cerri wrote:
> >
> > This is probably caused by the way that the xts template is handling the
> > underline algorithm selection.
> 
> Good catch.  I think the bigger issue here is that when requesting
> for an XTS that doesn't need a fallback we shouldn't be using an
> underlying ECB that needs one.  So this patch should fix the hang.

Yeah, I agree. This should work as long as the module aliases are
correct, which is enough.

Other templates will not trigger the same error since they don't have to
try more than one underlying algorithm. But I think this is still
desirable for the remaining templates to avoid a long chain of unused
fallbacks as in the example I gave in my previous email.

Probably a helper function to return the correct mask might be useful
for readability and to avoid duplicate code.

> 
> Thanks,
> 
> ---8<---
> Subject: crypto: xts - Propagate NEED_FALLBACK bit
> 
> When we're used as a fallback algorithm, we should propagate
> the NEED_FALLBACK bit when searching for the underlying ECB mode.
> 
> This just happens to fix a hang too because otherwise the search
> may end up loading the same module that triggered this XTS creation.
> 
> Fixes: f1c131b45410 ("crypto: xts - Convert to skcipher")
> Reported-by: Harald Freudenberger <freude@linux.vnet.ibm.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/crypto/xts.c b/crypto/xts.c
> index 410a2e2..2066161 100644
> --- a/crypto/xts.c
> +++ b/crypto/xts.c
> @@ -463,6 +463,7 @@ static int create(struct crypto_template *tmpl, struct rtattr **tb)
>  	struct xts_instance_ctx *ctx;
>  	struct skcipher_alg *alg;
>  	const char *cipher_name;
> +	u32 mask;
>  	int err;
>  
>  	algt = crypto_get_attr_type(tb);
> @@ -483,18 +484,19 @@ static int create(struct crypto_template *tmpl, struct rtattr **tb)
>  	ctx = skcipher_instance_ctx(inst);
>  
>  	crypto_set_skcipher_spawn(&ctx->spawn, skcipher_crypto_instance(inst));
> -	err = crypto_grab_skcipher(&ctx->spawn, cipher_name, 0,
> -				   crypto_requires_sync(algt->type,
> -							algt->mask));
> +
> +	mask = crypto_requires_sync(algt->type, algt->mask) |
> +	       ((algt->type ^ CRYPTO_ALG_NEED_FALLBACK) & algt->mask &
> +	        CRYPTO_ALG_NEED_FALLBACK);
> +
> +	err = crypto_grab_skcipher(&ctx->spawn, cipher_name, 0, mask);
>  	if (err == -ENOENT) {
>  		err = -ENAMETOOLONG;
>  		if (snprintf(ctx->name, CRYPTO_MAX_ALG_NAME, "ecb(%s)",
>  			     cipher_name) >= CRYPTO_MAX_ALG_NAME)
>  			goto err_free_inst;
>  
> -		err = crypto_grab_skcipher(&ctx->spawn, ctx->name, 0,
> -					   crypto_requires_sync(algt->type,
> -								algt->mask));
> +		err = crypto_grab_skcipher(&ctx->spawn, ctx->name, 0, mask);
>  	}
>  
>  	if (err)
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff mbox

Patch

diff --git a/crypto/xts.c b/crypto/xts.c
index 410a2e2..2066161 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -463,6 +463,7 @@  static int create(struct crypto_template *tmpl, struct rtattr **tb)
 	struct xts_instance_ctx *ctx;
 	struct skcipher_alg *alg;
 	const char *cipher_name;
+	u32 mask;
 	int err;
 
 	algt = crypto_get_attr_type(tb);
@@ -483,18 +484,19 @@  static int create(struct crypto_template *tmpl, struct rtattr **tb)
 	ctx = skcipher_instance_ctx(inst);
 
 	crypto_set_skcipher_spawn(&ctx->spawn, skcipher_crypto_instance(inst));
-	err = crypto_grab_skcipher(&ctx->spawn, cipher_name, 0,
-				   crypto_requires_sync(algt->type,
-							algt->mask));
+
+	mask = crypto_requires_sync(algt->type, algt->mask) |
+	       ((algt->type ^ CRYPTO_ALG_NEED_FALLBACK) & algt->mask &
+	        CRYPTO_ALG_NEED_FALLBACK);
+
+	err = crypto_grab_skcipher(&ctx->spawn, cipher_name, 0, mask);
 	if (err == -ENOENT) {
 		err = -ENAMETOOLONG;
 		if (snprintf(ctx->name, CRYPTO_MAX_ALG_NAME, "ecb(%s)",
 			     cipher_name) >= CRYPTO_MAX_ALG_NAME)
 			goto err_free_inst;
 
-		err = crypto_grab_skcipher(&ctx->spawn, ctx->name, 0,
-					   crypto_requires_sync(algt->type,
-								algt->mask));
+		err = crypto_grab_skcipher(&ctx->spawn, ctx->name, 0, mask);
 	}
 
 	if (err)