RE: [EXT] [PATCH] crypto: caam - fix double-free in caam_rsa_set_priv_key_form()

[Sahil Malhotra (OSS)]

Thanks, ZhaoJinming for the fix.

The fix looks correct.

Since RSA key form 3 support was later added by
4a651b122adb ("crypto: caam - add support for RSA key form 3"),
does this double-free affect both form 2 and form 3 parsing paths?
If so, it may be worth mentioning that in the commit message, although
the root cause still appears to have been introduced by 52e26d77b8b3
("crypto: caam - add support for RSA key form 2") .

Reviewed-by: Sahil Malhotra <sahil.malhotra@xxxxxxx>


NXP Confidential
> -----Original Message-----
> From: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> Sent: 23 September 2026 15:45
> To: Horia Geanta <horia.geanta@xxxxxxx>; Pankaj Gupta
> <pankaj.gupta@xxxxxxx>; Gaurav Jain <gaurav.jain@xxxxxxx>; Herbert Xu
> <herbert@xxxxxxxxxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>
> Cc: Tudor Ambarus <tudor-dan.ambarus@xxxxxxx>; Radu Andrei Alexe
> <radu.alexe@xxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; ZhaoJinming
> <zhaojinming@xxxxxxxxxxxxx>
> Subject: [EXT] [PATCH] crypto: caam - fix double-free in
> caam_rsa_set_priv_key_form()
>
> 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_rsa_set_priv_key_form() frees the partially allocated key components
> (p, q, tmp1, tmp2, dp, dq) with kfree_sensitive() on its error path, but does
> not set those pointers to NULL afterward.
>
> When the function returns an error, caam_rsa_set_priv_key() jumps to its err
> label and calls caam_rsa_free_key(), which frees all of those same pointers
> again, resulting in a double-free.
>
> This is reachable when the allocation of q, tmp1, tmp2, dp, dq or qinv fails,
> e.g. under memory pressure, or for a malformed key whose CRT members
> decode to zero length and make caam_read_rsa_crt() return NULL.
>
> Set each pointer to NULL right after freeing it so that the subsequent
> caam_rsa_free_key() call becomes a no-op for the already-freed fields.
>
> Fixes: 52e26d77b8b3 ("crypto: caam - add support for RSA key form 2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> ---
>  drivers/crypto/caam/caampkc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> index cb001aa1de66..ac515ed8c575 100644
> --- a/drivers/crypto/caam/caampkc.c
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -1033,16 +1033,22 @@ static int caam_rsa_set_priv_key_form(struct
> caam_rsa_ctx *ctx,
>
>  free_dq:
>         kfree_sensitive(rsa_key->dq);
> +       rsa_key->dq = NULL;
>  free_dp:
>         kfree_sensitive(rsa_key->dp);
> +       rsa_key->dp = NULL;
>  free_tmp2:
>         kfree_sensitive(rsa_key->tmp2);
> +       rsa_key->tmp2 = NULL;
>  free_tmp1:
>         kfree_sensitive(rsa_key->tmp1);
> +       rsa_key->tmp1 = NULL;
>  free_q:
>         kfree_sensitive(rsa_key->q);
> +       rsa_key->q = NULL;
>  free_p:
>         kfree_sensitive(rsa_key->p);
> +       rsa_key->p = NULL;
>         return -ENOMEM;
>  }
>
> --
> 2.51.0
>