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

Reply via email to