Skip to contentSkip to conversation

crypto: caam - fix double-free in caam_rsa_set_priv_key_form()

PATCH In review ZhaoJinming 3 participants 3 messages Sep 23 – Oct 2

Subsystems (3)
Stable branch Crypto API Freescale CAAM (Cryptographic Acceleration and Assurance Module) driver
lore
Share & tools
lore

ZhaoJinming

author
#0 lore raw

PATCH 1/1 crypto: caam - fix double-free in caam_rsa_set_priv_key_form()

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 [email protected]
Signed-off-by ZhaoJinming <[email protected]>
---
 drivers/crypto/caam/caampkc.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/crypto/caam/caampkc.c +6 -0
index cb001aa1de66..ac515ed8c575 100644
@@ -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;
 }
Signature · 1 line
2.51.0
Reviewed-by Sahil Malhotra on v1 1/1
#1 lore raw

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 <[email protected]>

NXP Confidential

Quoted 71 lines

-----Original Message-----
From: ZhaoJinming <[email protected]>
Sent: 23 September 2026 15:45
To: Horia Geanta <[email protected]>; Pankaj Gupta <[email protected]>; Gaurav Jain <[email protected]>; Herbert Xu <[email protected]>; David S . Miller <[email protected]>
Cc: Tudor Ambarus <[email protected]>; Radu Andrei Alexe <[email protected]>; [email protected]; linux- [email protected]; [email protected]; ZhaoJinming <[email protected]>
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 [email protected]
Signed-off-by ZhaoJinming <[email protected]>
---
 drivers/crypto/caam/caampkc.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/crypto/caam/caampkc.c +6 -1
index cb001aa1de66..ac515ed8c575 100644
@@ -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

maintainer
#2 lore raw
In reply to ZhaoJinming
Quoted 15 lines · ZhaoJinming

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.

Quoted 6 lines
Fixes 52e26d77b8b3 ("crypto: caam - add support for RSA key form 2")
Cc [email protected]
Signed-off-by ZhaoJinming <[email protected]>
---
 drivers/crypto/caam/caampkc.c | 6 ++++++
 1 file changed, 6 insertions(+)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt