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

[Herbert Xu]

On Mon, Apr 16, 2018 at 12:34:45PM +0300, 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ă 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Martin Townsend]

On Mon, Apr 16, 2018 at 10:34 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ă 
> ---
>  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);
>
> src_nents = sg_nents_for_len(req->src, req->src_len);
> dst_nents = sg_nents_for_len(req->dst, req->dst_len);
> @@ -953,6 +1006,7 @@ static struct akcipher_alg caam_rsa = {
> .max_size = caam_rsa_max_size,
> .init = caam_rsa_init_tfm,
> .exit = caam_rsa_exit_tfm,
> +   .reqsize = sizeof(struct caam_rsa_req_ctx),
> .base = {
> .cra_name = "rsa",
> .cra_driver_name = "rsa-caam",
> diff --git a/drivers/crypto/caam/caampkc.h b/drivers/crypto/caam/caampkc.h
> index fd145c46eae1..82645bcf8b27 100644
> --- a/drivers/crypto/caam/caampkc.h
> +++ b/drivers/crypto/caam/caampkc.h
> @@ -95,6 +95,14 @@ struct caam_rsa_ctx {
> struct device *dev;
>  };
>
> +/**
> + * caam_rsa_req_ctx - per request context.
> + * @src: input scatterlist (stripped of leading zeros)
> + */
> +struct caam_rsa_req_ctx {
> +   struct scatterlist src[2];
> +};
> +
>  /**
>   * rsa_edesc - s/w-extended rsa descriptor
>   * @src_nents : number of segments in input scatterlist
> --
> 2.16.2
>

Using linux-imx 4.9 I can confirm that with the patch it boots to the
prompt and IMA/EVM is happily measuring files.

Tested-by: Martin Townsend