On Thu, Feb 23, 2017 at 07:39:09PM +0800, Herbert Xu wrote:
> On Thu, Feb 23, 2017 at 07:19:57PM +0800, Herbert Xu wrote:
> > Harald Freudenberger <freude@linux.vnet.ibm.com> wrote:
> > > 
> > > 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?

This is probably caused by the way that the xts template is handling the
underline algorithm selection.

In the create function in crypto/xts.c:

	static int create(struct crypto_template *tmpl, struct rtattr **tb)
	{
	...
		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));
		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));
		}
	...

Then when the aes_s390 driver tries to allocate its fallback based on
its name ("xts(aes)"), the xts template will first look for an skcipher
"aes" algorithm, that doesn't exist. And because of that
crypto_larval_lookup will try to load the correspondent alias. Also,
since the template does not use the requested flag
CRYPTO_ALG_NEED_FALLBACK when it selects the underline algorithm, the
-all alias is also requested.

A simple workaround is to try the ecb algorithm before the original
algorithm. However this still can lead to the same problem when no ecb
implementation is available, not even by the driver itself.

Another improvement that can be useful is to honor the requested
CRYPTO_ALG_NEED_FALLBACK flag when selecting the underline algorithm.
It's very likely that the aes_s390 driver will end up using the
following chain of fallback algorithms:

xts-aes-s390 -> xts(ecb-aes-s390) -> xts(ecb(aes-s390)) ->
xts(ecb(aes-generic))

A similar scenario occurs for the vmx-crypto driver.

> 
> 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 <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/1111
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-- 
Regards,
Marcelo