Re: [PATCH] crypto: caam - fix concurrency issue in givencrypt descriptor

From: Herbert Xu <herbert@gondor.apana.org.au>
To: "Horia Geantă" <horia.geanta@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Aymen Sghaier <aymen.sghaier@nxp.com>,
	Valentin Ciocoi Radulescu <valentin.ciocoi@nxp.com>,
	Vakul Garg <vakul.garg@nxp.com>,
	Alexandru Porosanu <alexandru.porosanu@nxp.com>,
	linux-crypto@vger.kernel.org, NXP Linux Team <linux-imx@nxp.com>
Subject: Re: [PATCH] crypto: caam - fix concurrency issue in givencrypt descriptor
Date: Fri, 9 Aug 2019 16:16:26 +1000	[thread overview]
Message-ID: <20190809061626.GE10392@gondor.apana.org.au> (raw)
In-Reply-To: <20190730054833.24192-1-horia.geanta@nxp.com>

On Tue, Jul 30, 2019 at 08:48:33AM +0300, Horia Geantă wrote:
> IV transfer from ofifo to class2 (set up at [29][30]) is not guaranteed
> to be scheduled before the data transfer from ofifo to external memory
> (set up at [38]:
> 
> [29] 10FA0004           ld: ind-nfifo (len=4) imm
> [30] 81F00010               <nfifo_entry: ofifo->class2 type=msg len=16>
> [31] 14820004           ld: ccb2-datasz len=4 offs=0 imm
> [32] 00000010               data:0x00000010
> [33] 8210010D    operation: cls1-op aes cbc init-final enc
> [34] A8080B04         math: (seqin + math0)->vseqout len=4
> [35] 28000010    seqfifold: skip len=16
> [36] A8080A04         math: (seqin + math0)->vseqin len=4
> [37] 2F1E0000    seqfifold: both msg1->2-last2-last1 len=vseqinsz
> [38] 69300000   seqfifostr: msg len=vseqoutsz
> [39] 5C20000C      seqstr: ccb2 ctx len=12 offs=0
> 
> If ofifo -> external memory transfer happens first, DECO will hang
> (issuing a Watchdog Timeout error, if WDOG is enabled) waiting for
> data availability in ofifo for the ofifo -> c2 ififo transfer.
> 
> Make sure IV transfer happens first by waiting for all CAAM internal
> transfers to end before starting payload transfer.
> 
> New descriptor with jump command inserted at [37]:
> 
> [..]
> [36] A8080A04         math: (seqin + math0)->vseqin len=4
> [37] A1000401         jump: jsl1 all-match[!nfifopend] offset=[01] local->[38]
> [38] 2F1E0000    seqfifold: both msg1->2-last2-last1 len=vseqinsz
> [39] 69300000   seqfifostr: msg len=vseqoutsz
> [40] 5C20000C      seqstr: ccb2 ctx len=12 offs=0
> 
> [Note: the issue is present in the descriptor from the very beginning
> (cf. Fixes tag). However I've marked it v4.19+ since it's the oldest
> maintained kernel that the patch applies clean against.]
> 
> Cc: <stable@vger.kernel.org> # v4.19+
> Fixes: 1acebad3d8db8 ("crypto: caam - faster aead implementation")
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
> ---
>  drivers/crypto/caam/caamalg_desc.c | 9 +++++++++
>  drivers/crypto/caam/caamalg_desc.h | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
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