Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Iuliana Prodan <iuliana.prodan@nxp.com>
To:     Zhenzhong Duan <zhenzhong.duan@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>
CC:     Horia Geanta <horia.geanta@nxp.com>,
        Aymen Sghaier <aymen.sghaier@nxp.com>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH] crypto: caam - fix use after free issue in *_crypt_done
Thread-Topic: [PATCH] crypto: caam - fix use after free issue in *_crypt_done
Thread-Index: AQHWHRvtT9LOWs0K8UK7DEdR/IFCQg==
Date:   Tue, 28 Apr 2020 07:48:26 +0000
Message-ID: <VI1PR0402MB3712A2A328540479261ACCA68CAC0@VI1PR0402MB3712.eurprd04.prod.outlook.com>
References: <20200428051427.508-1-zhenzhong.duan@gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 1722f9c3-747f-41e8-6bb7-08d7eb4888c7
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Apr 2020 07:48:26.0098
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: XziSfff3+Rvm1QW4jr3KjjkhQS4hZTl9ZG5zCVQZAir6huRQYXvDuv2Co9QighcVmhzNwcvTjaKHpafl6WB24Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2799
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 4/28/2020 8:14 AM, Zhenzhong Duan wrote:=0A=
> In both aead_crypt_done and skcipher_crypt_done, edesc->bklog is=0A=
> referenced after the structure pointed by edesc is freed.=0A=
> =0A=
> Fix them by moving kfree(edesc) to the end of function call.=0A=
> =0A=
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>=0A=
=0A=
These issues were already fixed, and applied on cryptodev, by this =0A=
series: https://patchwork.kernel.org/cover/11476799/=0A=
=0A=
Regards,=0A=
Iulia=0A=
=0A=
> ---=0A=
>   drivers/crypto/caam/caamalg.c | 8 ++++----=0A=
>   1 file changed, 4 insertions(+), 4 deletions(-)=0A=
> =0A=
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.=
c=0A=
> index b7bb7c30adeb..6d746ef5e650 100644=0A=
> --- a/drivers/crypto/caam/caamalg.c=0A=
> +++ b/drivers/crypto/caam/caamalg.c=0A=
> @@ -973,8 +973,6 @@ static void aead_crypt_done(struct device *jrdev, u32=
 *desc, u32 err,=0A=
>   =0A=
>   	aead_unmap(jrdev, edesc, req);=0A=
>   =0A=
> -	kfree(edesc);=0A=
> -=0A=
>   	/*=0A=
>   	 * If no backlog flag, the completion of the request is done=0A=
>   	 * by CAAM, not crypto engine.=0A=
> @@ -983,6 +981,8 @@ static void aead_crypt_done(struct device *jrdev, u32=
 *desc, u32 err,=0A=
>   		aead_request_complete(req, ecode);=0A=
>   	else=0A=
>   		crypto_finalize_aead_request(jrp->engine, req, ecode);=0A=
> +=0A=
> +	kfree(edesc);=0A=
>   }=0A=
>   =0A=
>   static void skcipher_crypt_done(struct device *jrdev, u32 *desc, u32 er=
r,=0A=
> @@ -1022,8 +1022,6 @@ static void skcipher_crypt_done(struct device *jrde=
v, u32 *desc, u32 err,=0A=
>   		     DUMP_PREFIX_ADDRESS, 16, 4, req->dst,=0A=
>   		     edesc->dst_nents > 1 ? 100 : req->cryptlen, 1);=0A=
>   =0A=
> -	kfree(edesc);=0A=
> -=0A=
>   	/*=0A=
>   	 * If no backlog flag, the completion of the request is done=0A=
>   	 * by CAAM, not crypto engine.=0A=
> @@ -1032,6 +1030,8 @@ static void skcipher_crypt_done(struct device *jrde=
v, u32 *desc, u32 err,=0A=
>   		skcipher_request_complete(req, ecode);=0A=
>   	else=0A=
>   		crypto_finalize_skcipher_request(jrp->engine, req, ecode);=0A=
> +=0A=
> +	kfree(edesc);=0A=
>   }=0A=
>   =0A=
>   /*=0A=
> =0A=
=0A=