Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Mar 2022 20:54:58 +0100
From:   LABBE Corentin <clabbe@baylibre.com>
To:     John Keeping <john@metanate.com>
Cc:     heiko@sntech.de, herbert@gondor.apana.org.au, robh+dt@kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v2 06/18] crypto: rockchip: add fallback for cipher
Message-ID: <YiEdEoX79kDp8kUY@Red>
References: <20220302211113.4003816-1-clabbe@baylibre.com>
 <20220302211113.4003816-7-clabbe@baylibre.com>
 <YiDO8Tt9Lhx530Oz@donbot>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YiDO8Tt9Lhx530Oz@donbot>
Precedence: bulk

Le Thu, Mar 03, 2022 at 02:21:37PM +0000, John Keeping a ?crit :
> On Wed, Mar 02, 2022 at 09:11:01PM +0000, Corentin Labbe wrote:
> > The hardware does not handle 0 size length request, let's add a
> > fallback.
> > Furthermore fallback will be used for all unaligned case the hardware
> > cannot handle.
> > 
> > Fixes: ce0183cb6464b ("crypto: rockchip - switch to skcipher API")
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> >  drivers/crypto/rockchip/rk3288_crypto.h       |  2 +
> >  .../crypto/rockchip/rk3288_crypto_skcipher.c  | 97 ++++++++++++++++---
> >  2 files changed, 86 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
> > index c919d9a43a08..8b1e15d8ddc6 100644
> > --- a/drivers/crypto/rockchip/rk3288_crypto.h
> > +++ b/drivers/crypto/rockchip/rk3288_crypto.h
> > @@ -246,10 +246,12 @@ struct rk_cipher_ctx {
> >  	struct rk_crypto_info		*dev;
> >  	unsigned int			keylen;
> >  	u8				iv[AES_BLOCK_SIZE];
> > +	struct crypto_skcipher *fallback_tfm;
> >  };
> >  
> >  struct rk_cipher_rctx {
> >  	u32				mode;
> > +	struct skcipher_request fallback_req;   // keep at the end
> >  };
> >  
> >  enum alg_type {
> > diff --git a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
> > index bbd0bf52bf07..bf9d398cc54c 100644
> > --- a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
> > +++ b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
> > @@ -13,6 +13,63 @@
> >  
> >  #define RK_CRYPTO_DEC			BIT(0)
> >  
> > +static int rk_cipher_need_fallback(struct skcipher_request *req)
> > +{
> > +	struct scatterlist *sgs, *sgd;
> > +
> > +	if (!req->cryptlen)
> > +		return true;
> > +
> > +	sgs = req->src;
> > +	while (sgs) {
> > +		if (!IS_ALIGNED(sgs->offset, sizeof(u32))) {
> > +			return true;
> > +		}
> > +		if (sgs->length % 16) {
> 
> Can this be relaxed to check for alignment to 4 rather than 16?  That's
> the requirement for programming the registers.

No we cannot, the hardware could operate only one SG at a time, and the cipher operation need to be complete, so the length should be a multiple of AES_BLOCK_SIZE.
The original driver already have this size check.
But for DES/3DES this check is bad and should be 8, so a fix is needed anyway.

> 
> But I think this check is wrong in general as it doesn't account for
> cryptlen; with fscrypt I'm seeing sgs->length == 255 but cryptlen == 16
> so the hardware can be used but at the moment the fallback path is
> triggered.

Yes, I need to check min(sg->length, cryptlen_remaining) instead.
I will fix that.

Thanks.