Hi, On 9/2/20 4:10 PM, Van Leeuwen, Pascal wrote: >> -----Original Message----- >> From: linux-crypto-owner@xxxxxxxxxxxxxxx <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf Of Denis Efremov >> Sent: Thursday, August 27, 2020 8:44 AM >> To: linux-crypto@xxxxxxxxxxxxxxx >> Cc: Denis Efremov <efremov@xxxxxxxxx>; Corentin Labbe <clabbe.montjoie@xxxxxxxxx>; Herbert Xu >> <herbert@xxxxxxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx >> 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@xxxxxxxxx> >> --- >> 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
