Re: crypto: hang in crypto_larval_lookup
On 02/26/2017 05:22 AM, Herbert Xu wrote: > On Sat, Feb 25, 2017 at 04:20:22PM -0300, Marcelo Cerri wrote: >> 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. > You're right. Here is a patch to add a helper for this. > Thanks! > > ---8<--- > Subject: crypto: api - Add crypto_requires_off helper > > This patch adds crypto_requires_off which is an extension of > crypto_requires_sync for similar bits such as NEED_FALLBACK. > > Suggested-by: Marcelo Cerri> Signed-off-by: Herbert Xu > > diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h > index ebe4ded..436c4c2 100644 > --- a/include/crypto/algapi.h > +++ b/include/crypto/algapi.h > @@ -360,13 +360,18 @@ static inline struct crypto_alg > *crypto_get_attr_alg(struct rtattr **tb, > return crypto_attr_alg(tb[1], type, mask); > } > > +static inline int crypto_requires_off(u32 type, u32 mask, u32 off) > +{ > + return (type ^ off) & mask & off; > +} > + > /* > * Returns CRYPTO_ALG_ASYNC if type/mask requires the use of sync algorithms. > * Otherwise returns zero. > */ > static inline int crypto_requires_sync(u32 type, u32 mask) > { > - return (type ^ CRYPTO_ALG_ASYNC) & mask & CRYPTO_ALG_ASYNC; > + return crypto_requires_off(type, mask, CRYPTO_ALG_ASYNC); > } > > noinline unsigned long __crypto_memneq(const void *a, const void *b, size_t > size); applied the xts.c create patch v2 and the helper patch, built and installed. Now the aes_s390 module loads perfect without any hang, no complains in syslog and /proc/crypto shows that all selftests for the algs in the module passed successful. Thanks all for your help :-) regards, Harald Freudenberger
Re: crypto: hang in crypto_larval_lookup
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> Signed-off-by: Herbert Xu > > 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(>spawn, skcipher_crypto_instance(inst)); > - err = crypto_grab_skcipher(>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(>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(>spawn, ctx->name, 0, > -crypto_requires_sync(algt->type, > - algt->mask)); > + err = crypto_grab_skcipher(>spawn, ctx->name, 0, mask); > } > > if (err) > -- > Email: Herbert Xu > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- Regards, Marcelo signature.asc Description: PGP signature
Re: crypto: hang in crypto_larval_lookup
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 FreudenbergerSigned-off-by: Herbert Xu 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(>spawn, skcipher_crypto_instance(inst)); - err = crypto_grab_skcipher(>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(>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(>spawn, ctx->name, 0, - crypto_requires_sync(algt->type, - algt->mask)); + err = crypto_grab_skcipher(>spawn, ctx->name, 0, mask); } if (err) -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: crypto: hang in crypto_larval_lookup
On 02/24/2017 11:32 AM, Harald Freudenberger wrote: > On 02/24/2017 09:42 AM, Harald Freudenberger wrote: ... >> ... >> Feb 24 09:28:10 r35lp49 kernel: >> ->crypto_larval_lookup(name=aes,type=0x0405,mask=0x248c) >> Feb 24 09:28:10 r35lp49 kernel: crypto_larval_lookup calling >> crypto_alg_lookup(aes,0x0405,0x248c) >> Feb 24 09:28:10 r35lp49 kernel: ->__crypto_alg_lookup(name=aes) >> Feb 24 09:28:10 r35lp49 kernel: <-__crypto_alg_lookup(name=aes) alg= >> (null) >> Feb 24 09:28:10 r35lp49 kernel: crypto_larval_lookup calling >> request_module(crypto-aes) >> Feb 24 09:28:10 r35lp49 kernel: crypto_larval_lookup calling >> request_module(crypto-aes-all) >> >> type=0x0405 = CRYPTO_ALG_TESTED, CRYPTO_ALG_TYPE_SKCIPHER >> mask=0x248c = CRYPTO_ALG_INTERNAL, CRYPTO_ALG_TESTED, CRYPTO_ALG_ASYNC, >> CRYPTO_ALG_TYPE_BLKCIPHER_MASK >> > I catched it: Thanks Herbert for your hint. The aes algorith registers with: > .cra_flags = CRYPTO_ALG_TYPE_CIPHER |CRYPTO_ALG_NEED_FALLBACK > so later at __crypto_alg_lookup(aes, 0x0405, 0x248c) > this alg is not choosen because the check > if ((q->cra_flags ^ type) & mask) > is true q->cra_flags = 0x0501 ^ 0x0405 & 0x248x => 0x0004 > > @Martin, I'll commit a patch asap. > > Thanks for your help :-) > > regards, H.Freudenberger > rollback. Changing the cra_flag CRYPTO_ALG_TYPE_SKCIPHER in the crypto_alg registration struct is not the right way. continuing evaluation ...
Re: crypto: hang in crypto_larval_lookup
On 02/23/2017 12:39 PM, Herbert Xu wrote: > On Thu, Feb 23, 2017 at 07:19:57PM +0800, Herbert Xu wrote: >> Harald Freudenbergerwrote: >>> Hello all >>> >>> I am currently following a hang at modprobe aes_s390 where >>> crypto_register_alg() does not come back for the xts(aes) algorithm. >>> >>> The registration is waiting forever in algapi.c crypto_wait_for_test() but >>> the completion never occurs. The cryptomgr is triggering a test via >>> kthread_run to invoce cryptomgr_probe and this thread is calling the >>> create() function of the xts template (file xts.c). Following this thread >>> it comes down to api.c crypto_larval_lookup(name="aes") which is first >>> requesting the module "crypto-aes" via request_module() successful and then >>> blocking forever in requesting the module "crypto-aes-all". >>> >>> The xts(aes) has at registration CRYPTO_ALG_NEED_FALLBACK flag on. >>> >>> This problem is seen since about 6 weeks now, first only on the linux next >>> kernel. Now it appers on the 4.10-rc kernels as well. And I still have no >>> idea on how this could be fixed or what's wrong with just the xts >>> registration (ecb, cbc, ctr work fine). >>> >>> Any ideas or hints? >> Sorry, my fault. I should've converted all the fallback users of >> the old blkcipher interface over to skcipher before converting the >> core algorithms to skcipher. >> >> I'll send a patch. > Hmm, actually looks like I did convert this one :) > > Do you have ECB enabled in your configuration? XTS doesn't work > without it. Currently the Kconfig is missing a select on ECB so > it could stop the generic XTS from loading. > > However, you seem to be stuck on straight AES which quite strange. > The reason is that s390 crypto registers AES as the first thing so > it should already be available. > > The fact that it hangs is expected because it's trying to find > an acceptable AES implementation and in doing so it's loading > s390-aes again. > > So we need to get to the bottom of why there is no acceptable > "aes" registered. Can you check /proc/crypto to see if the simple > aes cipher is correctly registered (passing the selftest) after > it hangs? > > You could also print out the type/mask in crypto_larval_lookup > to see if perhaps the caller is asking for something unreasonable. > > Thanks, Thanks for your help, here is cat /proc/crypto: name : xts(aes) driver : module : kernel priority : -1 refcnt : 3 selftest : passed internal : no type : larval flags: 0x415 name : xts(aes) driver : xts-aes-s390 module : kernel priority : 400 refcnt : 1 selftest : passed internal : no type : larval flags: 0x514 name : xts(aes) driver : xts-aes-s390 module : aes_s390 priority : 400 refcnt : 4 selftest : unknown internal : no type : blkcipher blocksize: 16 min keysize : 32 max keysize : 64 ivsize : 16 geniv: ... name : ecb(aes) driver : ecb(aes-s390) module : kernel priority : 300 refcnt : 1 selftest : passed internal : no type : blkcipher blocksize: 16 min keysize : 16 max keysize : 32 ivsize : 0 geniv: name : ecb(aes) driver : ecb-aes-s390 module : aes_s390 priority : 400 refcnt : 1 selftest : passed internal : no type : blkcipher blocksize: 16 min keysize : 16 max keysize : 32 ivsize : 0 geniv: name : aes driver : aes-s390 module : aes_s390 priority : 300 refcnt : 1 selftest : passed internal : no type : cipher blocksize: 16 min keysize : 16 max keysize : 32 ... at the time where the modprobe hangs. And here is some syslog output (I added some printks for debugging) maybe it may give you some hints about whats going on during the modprobe: Feb 23 16:52:08 r35lp49 kernel: crypto_larval_lookup calling crypto_alg_lookup() Feb 23 16:52:08 r35lp49 kernel: ->__crypto_alg_lookup(name=aes) Feb 23 16:52:08 r35lp49 kernel: crypto_alg_get(alg.cra_name=aes) alg=00f2dd68 alg.cra_refcnt=2 Feb 23 16:52:08 r35lp49 kernel: crypto_mod_get(alg.cra_name=aes alg=00f2dd68 alg.cra_driver_name=aes-generic alg.cra_module.name=(null)) Feb 23 16:52:08 r35lp49 kernel: <-__crypto_alg_lookup(name=aes) alg=00f2dd68 Feb 23 16:52:08 r35lp49 kernel: <-crypto_larval_lookup(name=aes) alg=00f2dd68 Feb 23 16:52:08 r35lp49 kernel: crypto_mod_put(alg.cra_name=aes alg=00f2dd68 alg.cra_driver_name=aes-generic, alg.cra_module.name=(null)) Feb 23 16:52:08 r35lp49 kernel: crypto_alg_put(alg.cra_name=aes alg=00f2dd68 alg.cra_refcnt=2) Feb 23 16:52:08 r35lp49 kernel: crypto_mod_put(alg.cra_name=aes alg=03ff80178480 alg.cra_driver_name=aes-s390, alg.cra_module.name=aes_s390) Feb 23 16:52:08 r35lp49 kernel:
Re: crypto: hang in crypto_larval_lookup
Harald Freudenbergerwrote: > > Hello all > > I am currently following a hang at modprobe aes_s390 where > crypto_register_alg() does not come back for the xts(aes) algorithm. > > The registration is waiting forever in algapi.c crypto_wait_for_test() but > the completion never occurs. The cryptomgr is triggering a test via > kthread_run to invoce cryptomgr_probe and this thread is calling the > create() function of the xts template (file xts.c). Following this thread > it comes down to api.c crypto_larval_lookup(name="aes") which is first > requesting the module "crypto-aes" via request_module() successful and then > blocking forever in requesting the module "crypto-aes-all". > > The xts(aes) has at registration CRYPTO_ALG_NEED_FALLBACK flag on. > > This problem is seen since about 6 weeks now, first only on the linux next > kernel. Now it appers on the 4.10-rc kernels as well. And I still have no > idea on how this could be fixed or what's wrong with just the xts > registration (ecb, cbc, ctr work fine). > > Any ideas or hints? Sorry, my fault. I should've converted all the fallback users of the old blkcipher interface over to skcipher before converting the core algorithms to skcipher. I'll send a patch. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: crypto: hang in crypto_larval_lookup
On Thu, Feb 23, 2017 at 07:19:57PM +0800, Herbert Xu wrote: > Harald Freudenbergerwrote: > > > > Hello all > > > > I am currently following a hang at modprobe aes_s390 where > > crypto_register_alg() does not come back for the xts(aes) algorithm. > > > > The registration is waiting forever in algapi.c crypto_wait_for_test() but > > the completion never occurs. The cryptomgr is triggering a test via > > kthread_run to invoce cryptomgr_probe and this thread is calling the > > create() function of the xts template (file xts.c). Following this thread > > it comes down to api.c crypto_larval_lookup(name="aes") which is first > > requesting the module "crypto-aes" via request_module() successful and then > > blocking forever in requesting the module "crypto-aes-all". > > > > The xts(aes) has at registration CRYPTO_ALG_NEED_FALLBACK flag on. > > > > This problem is seen since about 6 weeks now, first only on the linux next > > kernel. Now it appers on the 4.10-rc kernels as well. And I still have no > > idea on how this could be fixed or what's wrong with just the xts > > registration (ecb, cbc, ctr work fine). > > > > Any ideas or hints? > > Sorry, my fault. I should've converted all the fallback users of > the old blkcipher interface over to skcipher before converting the > core algorithms to skcipher. > > I'll send a patch. Hmm, actually looks like I did convert this one :) Do you have ECB enabled in your configuration? XTS doesn't work without it. Currently the Kconfig is missing a select on ECB so it could stop the generic XTS from loading. However, you seem to be stuck on straight AES which quite strange. The reason is that s390 crypto registers AES as the first thing so it should already be available. The fact that it hangs is expected because it's trying to find an acceptable AES implementation and in doing so it's loading s390-aes again. So we need to get to the bottom of why there is no acceptable "aes" registered. Can you check /proc/crypto to see if the simple aes cipher is correctly registered (passing the selftest) after it hangs? You could also print out the type/mask in crypto_larval_lookup to see if perhaps the caller is asking for something unreasonable. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
crypto: hang in crypto_larval_lookup
Hello all I am currently following a hang at modprobe aes_s390 where crypto_register_alg() does not come back for the xts(aes) algorithm. The registration is waiting forever in algapi.c crypto_wait_for_test() but the completion never occurs. The cryptomgr is triggering a test via kthread_run to invoce cryptomgr_probe and this thread is calling the create() function of the xts template (file xts.c). Following this thread it comes down to api.c crypto_larval_lookup(name="aes") which is first requesting the module "crypto-aes" via request_module() successful and then blocking forever in requesting the module "crypto-aes-all". The xts(aes) has at registration CRYPTO_ALG_NEED_FALLBACK flag on. This problem is seen since about 6 weeks now, first only on the linux next kernel. Now it appers on the 4.10-rc kernels as well. And I still have no idea on how this could be fixed or what's wrong with just the xts registration (ecb, cbc, ctr work fine). Any ideas or hints? Thank's in advance. regards Harald Freudenberger