Hi, On 9/2/20 4:10 PM, Van Leeuwen, Pascal wrote: >> -----Original Message----- >> From: linux-crypto-ow...@vger.kernel.org >> <linux-crypto-ow...@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 <efre...@linux.com>; Corentin Labbe >> <clabbe.montj...@gmail.com>; Herbert Xu >> <herb...@gondor.apana.org.au>; linux-ker...@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 <efre...@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.
kfree_sensitive() uses memzero_explicit() internally. > 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. There are a number of predefined allocation sizes (power of 2) for faster alloc, i.e. https://elixir.bootlin.com/linux/latest/source/include/linux/slab.h#L349 and it looks like that keys we free in this patches are in bounds of these sizes. As far as I understand, if a key is not a power of 2 len, the buffer will be zeroed to the closest power of 2 size. For small sizes like these, performance difference should be unnoticeable because of cache lines and how arch-optimized memzero() works. Key freeing doesn't look like a frequent event. Thanks, Denis
