Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Thu, 21 Sep 2023 19:41:00 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Gaurav Jain <gaurav.jain@nxp.com>
Cc:     Horia Geanta <horia.geanta@nxp.com>,
        Pankaj Gupta <pankaj.gupta@nxp.com>,
        Varun Sethi <V.Sethi@nxp.com>,
        Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S . Miller" <davem@davemloft.net>,
        Aisheng Dong <aisheng.dong@nxp.com>,
        Silvano Di Ninno <silvano.dininno@nxp.com>,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-imx@nxp.com
Subject: Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test
 failure
Message-ID: <20230922024100.GA935@sol.localdomain>
References: <20230921124237.2364827-1-gaurav.jain@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230921124237.2364827-1-gaurav.jain@nxp.com>
Precedence: bulk

On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote:
> key buffer is not copied in chachapoly_setkey function,
> results in wrong output for encryption/decryption operation.
> 
> fix this by memcpy the key in caam_ctx key arrary
> 
> Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + Poly1305")
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> ---
>  drivers/crypto/caam/caamalg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index eba2d750c3b0..066f08a3a040 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead *aead, const u8 *key,
>  	if (keylen != CHACHA_KEY_SIZE + saltlen)
>  		return -EINVAL;
>  
> -	ctx->cdata.key_virt = key;
> +	memcpy(ctx->key, key, keylen);
> +	ctx->cdata.key_virt = ctx->key;
>  	ctx->cdata.keylen = keylen - saltlen;
>  

Huh, so this driver just ignored the key?  Is anyone using the ChaCha20Poly1305
support in this driver?  Based on this bug existing, that seems unlikely.  If
that's the case, wouldn't it be better just to remove the ChaCha20Poly1305
support from this driver so that the code doesn't need to be maintained?

- Eric