lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 11 Apr 2022 10:42:26 +0200 From: LABBE Corentin <clabbe@...libre.com> To: John Keeping <john@...anate.com> Cc: heiko@...ech.de, herbert@...dor.apana.org.au, krzk+dt@...nel.org, robh+dt@...nel.org, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org Subject: Re: [PATCH v4 06/33] crypto: rockchip: add fallback for cipher Le Mon, Apr 04, 2022 at 12:26:15PM +0100, John Keeping a écrit : > On Fri, Apr 01, 2022 at 08:17:37PM +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@...libre.com> > > --- > > diff --git a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c > > index bbd0bf52bf07..c6b601086c04 100644 > > --- a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c > > +++ b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c > > @@ -13,6 +13,71 @@ > > > > #define RK_CRYPTO_DEC BIT(0) > > > > +static int rk_cipher_need_fallback(struct skcipher_request *req) > > +{ > > + struct scatterlist *sgs, *sgd; > > + unsigned int todo, len; > > + unsigned int bs = crypto_skcipher_blocksize(tfm); > > + > > + if (!req->cryptlen) > > + return true; > > + > > + len = req->cryptlen; > > + sgs = req->src; > > + while (sgs) { > > + if (!IS_ALIGNED(sgs->offset, sizeof(u32))) { > > + return true; > > + } > > + todo = min(len, sgs->length); > > + if (todo % bs) { > > + return true; > > + } > > + len -= todo; > > + sgs = sg_next(sgs); > > + } > > + len = req->cryptlen; > > + sgd = req->dst; > > + while (sgd) { > > + if (!IS_ALIGNED(sgd->offset, sizeof(u32))) { > > + return true; > > + } > > + todo = min(len, sgd->length); > > + if (todo % bs) { > > + return true; > > + } > > + len -= todo; > > + sgd = sg_next(sgd); > > + } > > + sgs = req->src; > > + sgd = req->dst; > > + while (sgs && sgd) { > > + if (sgs->length != sgd->length) > > This check still seems to be triggering the fallback when it is not > needed. > > I've done some testing with fscrypt and the series is working great, but > the stats show the fallback triggering more than I'd expect. With some > extra logging here I see output like: > > sgs->length=32 sgd->length=255 req->cryptlen=16 > > In this case sgs and sgd are both the first (and only) entries in the > list. Should this take account of req->cryptlen as well? > > In fact, can't this whole function be folded into one loop over src and > dst at the same time, since all the checks must be the same? Something > like this (untested): > > while (sgs && sgd) { > if (!IS_ALIGNED(sgs->offset, sizeof(u32)) || > !IS_ALIGNED(sgd->offset, sizeof(u32))) > return true; > > todo = min(len, sgs->length); > if (todo % bs) > return true; > > if (sgd->length < todo) > return true; > > len -= todo; > sgs = sg_next(sgs); > sgd = sg_next(sgd); > } > > if (len) > return true; > Thanks, for this hint, I will use it. Regards
Powered by blists - more mailing lists