| Message ID | 20230613160723.61729-1-mngyadam@amazon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KEYS: use kfree_sensitive with key | expand |
On Tue, Jun 13, 2023 at 04:07:23PM +0000, Mahmoud Adam wrote: > key member might contain private part of the key, so better use > kfree_sensitive to free it > > Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> > --- > crypto/asymmetric_keys/public_key.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index eca5671ad3f2..006ae170a16f 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -43,7 +43,7 @@ static void public_key_describe(const struct key *asymmetric_key, > void public_key_free(struct public_key *key) > { > if (key) { > - kfree(key->key); > + kfree_sensitive(key->key); The public key should not be freed with kfree_sensitive. Cheers,
> On 14. Jun 2023, at 11:49, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Tue, Jun 13, 2023 at 04:07:23PM +0000, Mahmoud Adam wrote: >> key member might contain private part of the key, so better use >> kfree_sensitive to free it >> >> Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> >> --- >> crypto/asymmetric_keys/public_key.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c >> index eca5671ad3f2..006ae170a16f 100644 >> --- a/crypto/asymmetric_keys/public_key.c >> +++ b/crypto/asymmetric_keys/public_key.c >> @@ -43,7 +43,7 @@ static void public_key_describe(const struct key *asymmetric_key, >> void public_key_free(struct public_key *key) >> { >> if (key) { >> - kfree(key->key); >> + kfree_sensitive(key->key); > > The public key should not be freed with kfree_sensitive. I think this holds for the other lines as well, I can use pkey->key_is_private to check for them also Thanks. Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On Wed, Jun 14, 2023 at 01:32:51PM +0000, Adam, Mahmoud wrote: > > I think this holds for the other lines as well, I can use pkey->key_is_private to check for them also That might be going a bit overboard. So if the key is definitely public, then use kfree. If we don't know what it is (i.e., public or private), then just use kfree_sensitive. Thanks,
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index eca5671ad3f2..006ae170a16f 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -43,7 +43,7 @@ static void public_key_describe(const struct key *asymmetric_key, void public_key_free(struct public_key *key) { if (key) { - kfree(key->key); + kfree_sensitive(key->key); kfree(key->params); kfree(key); } @@ -218,7 +218,7 @@ static int software_key_query(const struct kernel_pkey_params *params, ret = 0; error_free_key: - kfree(key); + kfree_sensitive(key); error_free_tfm: crypto_free_akcipher(tfm); pr_devel("<==%s() = %d\n", __func__, ret); @@ -303,7 +303,7 @@ static int software_key_eds_op(struct kernel_pkey_params *params, ret = req->dst_len; error_free_key: - kfree(key); + kfree_sensitive(key); error_free_req: akcipher_request_free(req); error_free_tfm: @@ -456,7 +456,7 @@ int public_key_verify_signature(const struct public_key *pkey, ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait); error_free_key: - kfree(key); + kfree_sensitive(key); error_free_req: akcipher_request_free(req); error_free_tfm:
key member might contain private part of the key, so better use kfree_sensitive to free it Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> --- crypto/asymmetric_keys/public_key.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)