Re: [v2 PATCH 7/16] crypto: simd - Add simd skcipher helper

[Eric Biggers <ebiggers@google.com>]

On Sun, Nov 13, 2016 at 07:45:38PM +0800, Herbert Xu wrote:
> This patch adds the simd skcipher helper which is meant to be
> a replacement for ablk helper.  It replaces the underlying blkcipher
> interface with skcipher, and also presents the top-level algorithm
> as an skcipher.

I assume this means it's planned for all users of ablk_helper to be migrated to
crypto_simd, and ablk_helper will be removed?

> +	salg = kzalloc(sizeof(*alg), GFP_KERNEL);
> +	if (!salg) {
> +		salg = ERR_PTR(-ENOMEM);
> +		goto out_put_tfm;
> +	}

Shouldn't this be 'sizeof(*salg)'?

> +	tfm = crypto_alloc_skcipher(basename, CRYPTO_ALG_INTERNAL,
> +				    CRYPTO_ALG_INTERNAL | CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(tfm))
> +		return ERR_CAST(tfm);
> +
> +	ialg = crypto_skcipher_alg(tfm);

It seems this really just needs an algorithm and not a transform.  Perhaps it
should be calling crypto_find_alg() directly?

> +	err = -ENAMETOOLONG;
> +	if (snprintf(alg->base.cra_name, CRYPTO_MAX_ALG_NAME, "%s", algname) >=
> +	    CRYPTO_MAX_ALG_NAME)
> +		goto out_free_salg;
> +
> +	if (snprintf(alg->base.cra_driver_name, CRYPTO_MAX_ALG_NAME, "%s",
> +		     drvname) >= CRYPTO_MAX_ALG_NAME)
> +		goto out_free_salg;

Could use strscpy() or strlcpy() here.

> +static int simd_skcipher_encrypt(struct skcipher_request *req)
> +{
> +	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +	struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> +	struct skcipher_request *subreq;
> +	struct crypto_skcipher *child;
> +
> +	subreq = skcipher_request_ctx(req);
> +	*subreq = *req;
> +
> +	if (!may_use_simd() ||
> +	    (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
> +		child = &ctx->cryptd_tfm->base;
> +	else
> +		child = cryptd_skcipher_child(ctx->cryptd_tfm);
> +
> +	skcipher_request_set_tfm(subreq, child);
> +
> +	return crypto_skcipher_encrypt(subreq);
> +}
> +
> +static int simd_skcipher_decrypt(struct skcipher_request *req)
> +{
> +	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +	struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> +	struct skcipher_request *subreq;
> +	struct crypto_skcipher *child;
> +
> +	subreq = skcipher_request_ctx(req);
> +	*subreq = *req;
> +
> +	if (!may_use_simd() ||
> +	    (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
> +		child = &ctx->cryptd_tfm->base;
> +	else
> +		child = cryptd_skcipher_child(ctx->cryptd_tfm);
> +
> +	skcipher_request_set_tfm(subreq, child);
> +
> +	return crypto_skcipher_decrypt(subreq);
> +}

These are the same except for the
crypto_skcipher_encrypt/crypto_skcipher_decrypt at the end, so they could be
mostly shared.