Re: [PATCH v2 28/30] crypto: sa2ul - route requests through crypto_engine

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: sa2ul: unused-but-set variable enc_size in sa_aead_run()
--

commit f24f2777678674102188bb8e0c6e2374169ce53e
Author: Manorit Chawdhry <m-chawdhry@ti.com>

crypto: sa2ul - route requests through crypto_engine

This commit routes skcipher, ahash, and AEAD requests in the SA2UL driver
through crypto_engine so that request setup and submission run in a
sleepable context.

> diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
> index 19caaa41ba038..5716b44afaebf 100644
> --- a/drivers/crypto/sa2ul.c
> +++ b/drivers/crypto/sa2ul.c

[ ... ]

> @@ -1800,12 +1925,14 @@ static int sa_aead_cbc_sha256_setkey(struct crypto_aead *authenc,
>  	return sa_aead_setkey(authenc, key, keylen, &ad);
>  }
>  
> -static int sa_aead_run(struct aead_request *req, u8 *iv, int enc)
> +static int sa_aead_run(struct aead_request *req, int enc)
>  {
>  	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>  	struct sa_tfm_ctx *ctx = crypto_aead_ctx(tfm);
> -	struct sa_req sa_req = { 0 };
> +	struct sa_aead_req_ctx *rctx = aead_request_ctx(req);
>  	size_t auth_size, enc_size;
> +	unsigned long flags;
> +	int ret;
>  
>  	enc_size = req->cryptlen;
>  	auth_size = req->assoclen + req->cryptlen;
> @@ -1833,271 +1960,330 @@ static int sa_aead_run(struct aead_request *req, u8 *iv, int enc)
>  		return ret;
>  	}
>  
> +	rctx->enc = enc;
> +
> +	spin_lock_irqsave(&ctx->dev_data->request_lock, flags);
> +	ret = ctx->dev_data->stopping ? -ESHUTDOWN :
> +		crypto_transfer_aead_request_to_engine(ctx->dev_data->engine, req);
> +	spin_unlock_irqrestore(&ctx->dev_data->request_lock, flags);
> +	return ret;
> +}
> +
> +static int sa_aead_do_one_req(struct crypto_engine *engine, void *areq)
> +{
> +	struct aead_request *req = container_of(areq, struct aead_request, base);
> +	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> +	struct sa_tfm_ctx *ctx = crypto_aead_ctx(tfm);
> +	struct sa_aead_req_ctx *rctx = aead_request_ctx(req);
> +	struct sa_req sa_req = { 0 };
> +	size_t auth_size, enc_size;
> +
> +	enc_size = req->cryptlen;
> +	auth_size = req->assoclen + req->cryptlen;
> +
> +	if (!rctx->enc) {
> +		enc_size -= crypto_aead_authsize(tfm);
> +		auth_size -= crypto_aead_authsize(tfm);
> +	}
> +
>  	sa_req.enc_offset = req->assoclen;
>  	sa_req.enc_size = enc_size;

[Severity: Low]
Now that the struct sa_req setup has been moved to sa_aead_do_one_req(),
enc_size in sa_aead_run() is still assigned and decremented when !enc:

	enc_size = req->cryptlen;
	auth_size = req->assoclen + req->cryptlen;

	if (!enc) {
		enc_size -= crypto_aead_authsize(tfm);
		auth_size -= crypto_aead_authsize(tfm);
	}

but it is never read anywhere in sa_aead_run(). Does this trigger an
-Wunused-but-set-variable warning when building drivers/crypto/sa2ul.o
with W=1? Should enc_size be removed from sa_aead_run()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-b4-upstream-sa2ul-cleanup-v2-0-15ab90b3f9c2@ti.com?part=28