linux-kernel - Re: [PATCH v4 4/6] Add SPAcc ahash support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aKgwU4sVZrAHZ05s@gondor.apana.org.au>
Date: Fri, 22 Aug 2025 16:54:43 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Pavitrakumar Managutte <pavitrakumarm@...avyalabs.com>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, Ruud.Derwig@...opsys.com,
	manjunath.hadli@...avyalabs.com, adityak@...avyalabs.com,
	Bhoomika Kadabi <bhoomikak@...avyalabs.com>
Subject: Re: [PATCH v4 4/6] Add SPAcc ahash support

On Fri, Aug 08, 2025 at 05:56:29PM +0530, Pavitrakumar Managutte wrote:
>
> +static int spacc_hash_digest(struct ahash_request *req)
> +{
> +	int rc = 0;
> +	struct crypto_ahash *reqtfm = crypto_ahash_reqtfm(req);
> +	struct spacc_crypto_ctx *tctx = crypto_ahash_ctx(reqtfm);
> +	struct spacc_crypto_reqctx *ctx = ahash_request_ctx(req);
> +	struct spacc_priv *priv = dev_get_drvdata(tctx->dev);
> +	const struct spacc_alg *salg = spacc_tfm_ahash(&reqtfm->base);
> +
> +	/* direct single shot digest call */
> +	ctx->single_shot = 1;
> +	ctx->total_nents = sg_nents(req->src);
> +
> +	/* alloc tmp_sgl */
> +	tctx->tmp_sgl = kmalloc(sizeof(*tctx->tmp_sgl) * 2, GFP_KERNEL);
> +
> +	if (!tctx->tmp_sgl)
> +		return -ENOMEM;

You should use a software fallback to handle the failure case.

Also GFP_KERNEL cannot be used because this path may be called
from softirqs.  Please use GFP_ATOMIC.

> +	sg_init_table(tctx->tmp_sgl, 2);
> +	tctx->tmp_sgl[0].length = 0;
> +
> +	if (tctx->handle < 0 || !tctx->ctx_valid) {
> +		priv = NULL;
> +		priv = dev_get_drvdata(salg->dev);
> +		tctx->dev = get_device(salg->dev);
> +
> +		rc = spacc_is_mode_keysize_supported(&priv->spacc,
> +						     salg->mode->id, 0, 1);
> +		if (rc)
> +			tctx->handle = spacc_open(&priv->spacc,
> +						  CRYPTO_MODE_NULL,
> +						  salg->mode->id, -1, 0,
> +						  spacc_digest_cb,
> +						  reqtfm);

This thing needs to be redesigned.  The digest function can be
called from softirq context, so it must never sleep.  The function
spacc_open tries to obtain a mutex and this cannot possibly work.

I think you should hook this up to crypto_engine which would allow
your driver to sleep.

Another request is that your driver is too big to review.  Please
submit one functionality at a time so that it's easier for me.  Perhaps
start with just skcipher since that's where spacc_open is currently.

Cheers,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt