RE: [EXT] [REPOST PATCH] crypto: caam - reject overlong RSA CRT parameters

[Sahil Malhotra (OSS)]

Hi Jeremy,

Sorry for the delayed response.

I am ok with this patch.
Reviewed-by: Sahil Malhotra <sahil.malhotra@xxxxxxx>

Regards,
Sahil Malhotra


NXP Confidential
> -----Original Message-----
> From: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> Sent: 08 October 2026 15:06
> To: Horia Geanta <horia.geanta@xxxxxxx>; Pankaj Gupta
> <pankaj.gupta@xxxxxxx>; Sahil Malhotra <sahil.malhotra@xxxxxxx>;
> Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> Subject: [EXT] [REPOST PATCH] crypto: caam - reject overlong RSA CRT
> parameters
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> caam_read_rsa_crt() right-aligns dP, dQ, and qInv in buffers sized from the
> corresponding prime. rsa_parse_priv_key() only bounds these integers against
> n, so a malformed key can provide, for example, a two-byte dP with a one-
> byte p. dstlen - nbytes then underflows and
> memcpy() writes outside the allocation during set_priv_key(). KASAN reports
> a slab-out-of-bounds write in caam_read_rsa_crt().
>
> Reject empty or overlong CRT parameters after stripping leading zeros.
> qInv is consumed by CAAM as a p-sized value, so size it from p rather than q
> to match the DMA mapping in set_rsa_priv_f3_pdb(). Let the top-level key
> cleanup handle partial form-3 allocations and return the specific error to the
> caller.
>
> Also test nbytes before dereferencing it while stripping zeros, so an all-zero
> integer does not read one byte beyond its input.
>
> Fixes: 4a651b122adb ("crypto: caam - add support for RSA key form 3")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/crypto/caam/caampkc.c | 69 +++++++++++++++--------------------
>  1 file changed, 30 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> index cb001aa1de66..2082adaae585 100644
> --- a/drivers/crypto/caam/caampkc.c
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -880,7 +880,7 @@ static void caam_rsa_free_key(struct caam_rsa_key
> *key)
>
>  static void caam_rsa_drop_leading_zeros(const u8 **ptr, size_t *nbytes)  {
> -       while (!**ptr && *nbytes) {
> +       while (*nbytes && !**ptr) {
>                 (*ptr)++;
>                 (*nbytes)--;
>         }
> @@ -896,22 +896,22 @@ static void caam_rsa_drop_leading_zeros(const u8
> **ptr, size_t *nbytes)
>   * @ptr   : pointer to {dP, dQ, qInv} CRT member
>   * @nbytes: length in bytes of {dP, dQ, qInv} CRT member
>   * @dstlen: length in bytes of corresponding p or q prime factor
> + * @dst   : pointer to the zero-padded output buffer
>   */
> -static u8 *caam_read_rsa_crt(const u8 *ptr, size_t nbytes, size_t dstlen)
> +static int caam_read_rsa_crt(const u8 *ptr, size_t nbytes, size_t dstlen,
> +                            u8 **dst)
>  {
> -       u8 *dst;
> -
>         caam_rsa_drop_leading_zeros(&ptr, &nbytes);
> -       if (!nbytes)
> -               return NULL;
> +       if (!nbytes || nbytes > dstlen)
> +               return -EINVAL;
>
> -       dst = kzalloc(dstlen, GFP_KERNEL);
> -       if (!dst)
> -               return NULL;
> +       *dst = kzalloc(dstlen, GFP_KERNEL);
> +       if (!*dst)
> +               return -ENOMEM;
>
> -       memcpy(dst + (dstlen - nbytes), ptr, nbytes);
> +       memcpy(*dst + (dstlen - nbytes), ptr, nbytes);
>
> -       return dst;
> +       return 0;
>  }
>
>  /**
> @@ -991,6 +991,7 @@ static int caam_rsa_set_priv_key_form(struct
> caam_rsa_ctx *ctx,
>         size_t p_sz = raw_key->p_sz;
>         size_t q_sz = raw_key->q_sz;
>         unsigned aligned_size;
> +       int ret;
>
>         rsa_key->p = caam_read_raw_data(raw_key->p, &p_sz);
>         if (!rsa_key->p)
> @@ -999,51 +1000,39 @@ static int caam_rsa_set_priv_key_form(struct
> caam_rsa_ctx *ctx,
>
>         rsa_key->q = caam_read_raw_data(raw_key->q, &q_sz);
>         if (!rsa_key->q)
> -               goto free_p;
> +               return -ENOMEM;
>         rsa_key->q_sz = q_sz;
>
>         aligned_size = ALIGN(raw_key->p_sz, dma_get_cache_alignment());
>         rsa_key->tmp1 = kzalloc(aligned_size, GFP_KERNEL);
>         if (!rsa_key->tmp1)
> -               goto free_q;
> +               return -ENOMEM;
>
>         aligned_size = ALIGN(raw_key->q_sz, dma_get_cache_alignment());
>         rsa_key->tmp2 = kzalloc(aligned_size, GFP_KERNEL);
>         if (!rsa_key->tmp2)
> -               goto free_tmp1;
> +               return -ENOMEM;
>
>         rsa_key->priv_form = FORM2;
>
> -       rsa_key->dp = caam_read_rsa_crt(raw_key->dp, raw_key->dp_sz, p_sz);
> -       if (!rsa_key->dp)
> -               goto free_tmp2;
> +       ret = caam_read_rsa_crt(raw_key->dp, raw_key->dp_sz, p_sz,
> +                               &rsa_key->dp);
> +       if (ret)
> +               return ret;
>
> -       rsa_key->dq = caam_read_rsa_crt(raw_key->dq, raw_key->dq_sz, q_sz);
> -       if (!rsa_key->dq)
> -               goto free_dp;
> +       ret = caam_read_rsa_crt(raw_key->dq, raw_key->dq_sz, q_sz,
> +                               &rsa_key->dq);
> +       if (ret)
> +               return ret;
>
> -       rsa_key->qinv = caam_read_rsa_crt(raw_key->qinv, raw_key->qinv_sz,
> -                                         q_sz);
> -       if (!rsa_key->qinv)
> -               goto free_dq;
> +       ret = caam_read_rsa_crt(raw_key->qinv, raw_key->qinv_sz, p_sz,
> +                               &rsa_key->qinv);
> +       if (ret)
> +               return ret;
>
>         rsa_key->priv_form = FORM3;
>
>         return 0;
> -
> -free_dq:
> -       kfree_sensitive(rsa_key->dq);
> -free_dp:
> -       kfree_sensitive(rsa_key->dp);
> -free_tmp2:
> -       kfree_sensitive(rsa_key->tmp2);
> -free_tmp1:
> -       kfree_sensitive(rsa_key->tmp1);
> -free_q:
> -       kfree_sensitive(rsa_key->q);
> -free_p:
> -       kfree_sensitive(rsa_key->p);
> -       return -ENOMEM;
>  }
>
>  static int caam_rsa_set_priv_key(struct crypto_akcipher *tfm, const void *key,
> @@ -1061,6 +1050,8 @@ static int caam_rsa_set_priv_key(struct
> crypto_akcipher *tfm, const void *key,
>         if (ret)
>                 return ret;
>
> +       ret = -ENOMEM;
> +
>         /* Copy key in DMA zone */
>         rsa_key->d = kmemdup(raw_key.d, raw_key.d_sz, GFP_KERNEL);
>         if (!rsa_key->d)
> @@ -1097,7 +1088,7 @@ static int caam_rsa_set_priv_key(struct
> crypto_akcipher *tfm, const void *key,
>
>  err:
>         caam_rsa_free_key(rsa_key);
> -       return -ENOMEM;
> +       return ret;
>  }
>
>  static unsigned int caam_rsa_max_size(struct crypto_akcipher *tfm)
> --
> 2.47.3