linux-kernel - Re: [PATCH v2 1/4] crypto: inside-secure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8ac5cb88-7df5-1060-2fac-510541fca48b@linux.com>
Date:   Fri, 4 Sep 2020 11:55:26 +0300
From:   Denis Efremov <efremov@...ux.com>
To:     "Van Leeuwen, Pascal" <pvanleeuwen@...bus.com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>
Cc:     Corentin Labbe <clabbe.montjoie@...il.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] crypto: inside-secure - use kfree_sensitive()

Hi,

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