Re: [PATCH RESEND] crypto: caam - strip input zeros from RSA input buffer

[Tudor Ambarus]

On 04/16/2018 04:07 PM, Horia Geantă wrote:

Sometimes the provided RSA input buffer provided is not stripped
of leading zeros. This could cause its size to be bigger than that
of the modulus, making the HW complain:

caam_jr 2142000.jr1: 4789: DECO: desc idx 7:
Protocol Size Error - A protocol has seen an error in size. When
running RSA, pdb size N < (size of F) when no formatting is used; or
pdb size N < (F + 11) when formatting is used.

Fix the problem by stripping off the leading zero from input data
before feeding it to the CAAM accelerator.

Fixes: 8c419778ab57e ("crypto: caam - add support for RSA algorithm")
Cc:  # 4.8+
Reported-by: Martin Townsend 
Link: 
https://lkml.kernel.org/r/cabatt_ytyoryktapcb4izhnanekkgfi9xaqmjhi_n-8ywoc...@mail.gmail.com
Signed-off-by: Horia Geantă 
---
(Hopefully this one will reach the mailing list.
Sorry for the noise, problems with SMTP.)

  drivers/crypto/caam/caampkc.c | 54 +++
  drivers/crypto/caam/caampkc.h |  8 +++
  2 files changed, 62 insertions(+)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 7a897209f181..979072b25eaa 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -166,18 +166,71 @@ static void rsa_priv_f3_done(struct device *dev, u32 
*desc, u32 err,
akcipher_request_complete(req, err);
  }
  
+static int caam_rsa_count_leading_zeros(struct scatterlist *sgl,

+   unsigned int nbytes,
+   unsigned int flags)
+{
+   struct sg_mapping_iter miter;
+   int lzeros, ents;
+   unsigned int len;
+   unsigned int tbytes = nbytes;
+   const u8 *buff;
+
+   ents = sg_nents_for_len(sgl, nbytes);
+   if (ents < 0)
+   return ents;
+
+   sg_miter_start(, sgl, ents, SG_MITER_FROM_SG | flags);
+
+   lzeros = 0;
+   len = 0;
+   while (nbytes > 0) {
+   while (len && !*buff) {
+   lzeros++;
+   len--;
+   buff++;
+   }
+
+   if (len && *buff)
+   break;
+
+   sg_miter_next();
+   buff = miter.addr;
+   len = miter.length;
+
+   nbytes -= lzeros;
+   lzeros = 0;
+   }
+
+   miter.consumed = lzeros;
+   sg_miter_stop();
+   nbytes -= lzeros;
+
+   return tbytes - nbytes;
+}
+
  static struct rsa_edesc *rsa_edesc_alloc(struct akcipher_request *req,
 size_t desclen)
  {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
struct caam_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
struct device *dev = ctx->dev;
+   struct caam_rsa_req_ctx *req_ctx = akcipher_request_ctx(req);
struct rsa_edesc *edesc;
gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
   GFP_KERNEL : GFP_ATOMIC;
+   int sg_flags = (flags == GFP_ATOMIC) ? SG_MITER_ATOMIC : 0;
int sgc;
int sec4_sg_index, sec4_sg_len = 0, sec4_sg_bytes;
int src_nents, dst_nents;
+   int lzeros;
+
+   lzeros = caam_rsa_count_leading_zeros(req->src, req->src_len, sg_flags);
+   if (lzeros < 0)
+   return ERR_PTR(lzeros);
+
+   req->src_len -= lzeros;
+   req->src = scatterwalk_ffwd(req_ctx->src, req->src, lzeros);


This is interesting, you are overwriting the request's src sg. I kept
wondering if one could have a problem if we are modifying its src sg. I
couldn't find any failing scenario, so:

Reviewed-by: Tudor Ambarus 

Re: [PATCH RESEND] crypto: caam - strip input zeros from RSA input buffer

[Fabio Estevam]

On Mon, Apr 16, 2018 at 10:07 AM, Horia Geantă  wrote:
> Sometimes the provided RSA input buffer provided is not stripped
> of leading zeros. This could cause its size to be bigger than that
> of the modulus, making the HW complain:
>
> caam_jr 2142000.jr1: 4789: DECO: desc idx 7:
> Protocol Size Error - A protocol has seen an error in size. When
> running RSA, pdb size N < (size of F) when no formatting is used; or
> pdb size N < (F + 11) when formatting is used.
>
> Fix the problem by stripping off the leading zero from input data
> before feeding it to the CAAM accelerator.
>
> Fixes: 8c419778ab57e ("crypto: caam - add support for RSA algorithm")
> Cc:  # 4.8+
> Reported-by: Martin Townsend 
> Link: 
> https://lkml.kernel.org/r/cabatt_ytyoryktapcb4izhnanekkgfi9xaqmjhi_n-8ywoc...@mail.gmail.com
> Signed-off-by: Horia Geantă 

Tested-by: Fabio Estevam