Re: [PATCH v3 2/2] crypto: hisilicon/sec2 - fix for aead invalid authsize

Herbert Xu
2024-11-10T03:36:46Z

On Sat, Nov 02, 2024 at 10:55:59AM +0800, Chenghai Huang wrote:
>
> @@ -2226,15 +2236,15 @@ static int sec_aead_spec_check(struct sec_ctx *ctx, struct sec_req *sreq)
> struct device *dev = ctx->dev;
> int ret;
>
> - if (unlikely(req->cryptlen + req->assoclen > MAX_INPUT_DATA_LEN ||
> - req->assoclen > SEC_MAX_AAD_LEN)) {
> - dev_err(dev, "aead input spec error!\n");
> + /* Hardware does not handle cases where authsize is less than 4 bytes */
> + if (unlikely(sz < MIN_MAC_LEN)) {
> + ctx->a_ctx.fallback = true;

This is broken. sec_aead_spec_check is a per-request function,
called without any locking. Therefore it must not modify any
field in the tfm context (at least not without additional locking),
because multiple requests can be issued on the same tfm at any time.

I suppose for this field in particular you could move it to
set_authsize and there it would be safe to change the tfm context.

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