Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Horia Geanta <horia.geanta@nxp.com>
To:     Iuliana Prodan <iuliana.prodan@nxp.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Aymen Sghaier <aymen.sghaier@nxp.com>
CC:     "David S. Miller" <davem@davemloft.net>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH 03/14] crypto: caam - update IV only when crypto operation
 succeeds
Thread-Topic: [PATCH 03/14] crypto: caam - update IV only when crypto
 operation succeeds
Thread-Index: AQHVPXd3cwGqxppDGUKIXr9oiBjl2g==
Date:   Thu, 18 Jul 2019 14:59:37 +0000
Message-ID: <VI1PR0402MB34855E675A58E1221ACE7B9498C80@VI1PR0402MB3485.eurprd04.prod.outlook.com>
References: <1563461124-24641-1-git-send-email-iuliana.prodan@nxp.com>
 <1563461124-24641-4-git-send-email-iuliana.prodan@nxp.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: a7f5eb99-9415-4d65-e077-08d70b908d89
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Jul 2019 14:59:37.3724
 (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: horia.geanta@nxp.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB3648
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 7/18/2019 5:45 PM, Iuliana Prodan wrote:=0A=
> From: Horia Geant=E3 <horia.geanta@nxp.com>=0A=
> =0A=
> skcipher encryption might fail and in some cases, like (invalid) input=0A=
> length smaller then block size, updating the IV would lead to panic=0A=
> due to copying from a negative offset (req->cryptlen - ivsize).=0A=
> =0A=
The commit message is no longer in sync with the code base.=0A=
=0A=
More exactly, after commit=0A=
334d37c9e263 ("crypto: caam - update IV using HW support")=0A=
there shouldn't be any panic, only a useless IV copy in case HW issued=0A=
an error.=0A=
=0A=
> Signed-off-by: Horia Geant=E3 <horia.geanta@nxp.com>=0A=
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>=0A=
> ---=0A=
>  drivers/crypto/caam/caamalg.c     | 5 ++---=0A=
>  drivers/crypto/caam/caamalg_qi.c  | 4 +++-=0A=
>  drivers/crypto/caam/caamalg_qi2.c | 8 ++++++--=0A=
>  3 files changed, 11 insertions(+), 6 deletions(-)=0A=
> =0A=
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.=
c=0A=
> index 06b4f2d..28d55a0 100644=0A=
> --- a/drivers/crypto/caam/caamalg.c=0A=
> +++ b/drivers/crypto/caam/caamalg.c=0A=
> @@ -990,10 +990,9 @@ static void skcipher_encrypt_done(struct device *jrd=
ev, u32 *desc, u32 err,=0A=
>  	 * ciphertext block (CBC mode) or last counter (CTR mode).=0A=
>  	 * This is used e.g. by the CTS mode.=0A=
>  	 */=0A=
> -	if (ivsize) {=0A=
> +	if (ivsize && !ecode) {=0A=
>  		memcpy(req->iv, (u8 *)edesc->sec4_sg + edesc->sec4_sg_bytes,=0A=
>  		       ivsize);=0A=
> -=0A=
>  		print_hex_dump_debug("dstiv  @"__stringify(__LINE__)": ",=0A=
>  				     DUMP_PREFIX_ADDRESS, 16, 4, req->iv,=0A=
>  				     edesc->src_nents > 1 ? 100 : ivsize, 1);=0A=
> @@ -1030,7 +1029,7 @@ static void skcipher_decrypt_done(struct device *jr=
dev, u32 *desc, u32 err,=0A=
>  	 * ciphertext block (CBC mode) or last counter (CTR mode).=0A=
>  	 * This is used e.g. by the CTS mode.=0A=
>  	 */=0A=
> -	if (ivsize) {=0A=
> +	if (ivsize && !ecode) {=0A=
>  		memcpy(req->iv, (u8 *)edesc->sec4_sg + edesc->sec4_sg_bytes,=0A=
>  		       ivsize);=0A=
>  =0A=
> diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caama=
lg_qi.c=0A=
> index ab263b1..66531d6 100644=0A=
> --- a/drivers/crypto/caam/caamalg_qi.c=0A=
> +++ b/drivers/crypto/caam/caamalg_qi.c=0A=
> @@ -1201,7 +1201,9 @@ static void skcipher_done(struct caam_drv_req *drv_=
req, u32 status)=0A=
>  	 * ciphertext block (CBC mode) or last counter (CTR mode).=0A=
>  	 * This is used e.g. by the CTS mode.=0A=
>  	 */=0A=
> -	memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);=0A=
> +	if (!ecode)=0A=
> +		memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,=0A=
> +		       ivsize);=0A=
>  =0A=
>  	qi_cache_free(edesc);=0A=
>  	skcipher_request_complete(req, ecode);=0A=
> diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caam=
alg_qi2.c=0A=
> index 2681581..bc370af 100644=0A=
> --- a/drivers/crypto/caam/caamalg_qi2.c=0A=
> +++ b/drivers/crypto/caam/caamalg_qi2.c=0A=
> @@ -1358,7 +1358,9 @@ static void skcipher_encrypt_done(void *cbk_ctx, u3=
2 status)=0A=
>  	 * ciphertext block (CBC mode) or last counter (CTR mode).=0A=
>  	 * This is used e.g. by the CTS mode.=0A=
>  	 */=0A=
> -	memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);=0A=
> +	if (!ecode)=0A=
> +		memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,=0A=
> +		       ivsize);=0A=
>  =0A=
>  	qi_cache_free(edesc);=0A=
>  	skcipher_request_complete(req, ecode);=0A=
> @@ -1394,7 +1396,9 @@ static void skcipher_decrypt_done(void *cbk_ctx, u3=
2 status)=0A=
>  	 * ciphertext block (CBC mode) or last counter (CTR mode).=0A=
>  	 * This is used e.g. by the CTS mode.=0A=
>  	 */=0A=
> -	memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);=0A=
> +	if (!ecode)=0A=
> +		memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,=0A=
> +		       ivsize);=0A=
>  =0A=
>  	qi_cache_free(edesc);=0A=
>  	skcipher_request_complete(req, ecode);=0A=
> =0A=