RE: [PATCH v2 1/4] crypto: inside-secure - use kfree_sensitive()

["Van Leeuwen, Pascal" <pvanleeuwen@rambus.com>]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Denis Efremov
> Sent: Thursday, August 27, 2020 8:44 AM
> To: linux-crypto@vger.kernel.org
> Cc: Denis Efremov <efremov@linux.com>; Corentin Labbe <clabbe.montjoie@gmail.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; linux-kernel@vger.kernel.org
> Subject: [PATCH v2 1/4] crypto: inside-secure - use kfree_sensitive()
>
> <<< External Email >>>
> Use kfree_sensitive() instead of open-coding it.
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/crypto/inside-secure/safexcel_hash.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
> index 16a467969d8e..5ffdc1cd5847 100644
> --- a/drivers/crypto/inside-secure/safexcel_hash.c
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> @@ -1082,8 +1082,7 @@ static int safexcel_hmac_init_pad(struct ahash_request *areq,
>  }
>
>  /* Avoid leaking */
> -memzero_explicit(keydup, keylen);
> -kfree(keydup);
> +kfree_sensitive(keydup);
>
I'm not sure here ... I verified it does not break the driver (not a big surprise), but ...

memzero_explicit guarantees that it will not get optimized away and the keydata _always_
gets overwritten. Does kfree_sensitive also come with such a guarantee? I could not find a
hard statement on that in its documentation. Although the "sensitive" part surely suggests
it.

Additionally, this remark is made in the documentation for kfree_sensitive: "this function
zeroes the whole allocated buffer which can be a good deal bigger than the requested buffer
size passed to kmalloc().  So be careful when using this function in performance sensitive
code"

While the memzero_explicit does not zeroize anything beyond keylen.
Which is all you really need here, so why would you want to zeroize potentially a lot more?
In any case the two are not fully equivalent.

Any opinions?

>  if (ret)
>  return ret;
> --
> 2.26.2

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>