On Mon, Aug 25, 2014 at 10:01:39PM +0200, Daniel Borkmann wrote:
> zatimend has reported that in his environment (3.16/gcc4.8.3/corei7)
> memset() calls which clear out sensitive data in extract_{buf,entropy,
> entropy_user}() in random driver are being optimized away by gcc.
>
> Add a helper memzero_explicit() (similarly as explicit_bzero() variants)
> that can be used in such cases where a variable with sensitive data is
> being cleared out in the end. Other use cases might also be in crypto
> code. [ I have put this into lib/string.c though, as it's always built-in
> and doesn't need any dependencies then. ]
>
> Fixes kernel bugzilla: 82041
>
What???
That's not ok. We totally rely on memset to work. Every single memset
that I have added was absolutely necessary. This should be fixed
so that memset works instead of adding a work around for specific
drivers.
regards,
dan carpnter
On 12/01/2014 11:27 AM, Dan Carpenter wrote:
> On Mon, Aug 25, 2014 at 10:01:39PM +0200, Daniel Borkmann wrote:
>> zatimend has reported that in his environment (3.16/gcc4.8.3/corei7)
>> memset() calls which clear out sensitive data in extract_{buf,entropy,
>> entropy_user}() in random driver are being optimized away by gcc.
>>
>> Add a helper memzero_explicit() (similarly as explicit_bzero() variants)
>> that can be used in such cases where a variable with sensitive data is
>> being cleared out in the end. Other use cases might also be in crypto
>> code. [ I have put this into lib/string.c though, as it's always built-in
>> and doesn't need any dependencies then. ]
>>
>> Fixes kernel bugzilla: 82041
>
> What???
>
> That's not ok. We totally rely on memset to work. Every single memset
> that I have added was absolutely necessary. This should be fixed
> so that memset works instead of adding a work around for specific
> drivers.
Well, BSD has helpers such as bzero_explicit() for such cases to work
around this, which memzero_explicit() similarly does; see also [1].
[1] https://gcc.gnu.org/ml/gcc-help/2014-10/msg00059.html
On Mon, Dec 01, 2014 at 12:04:57PM +0100, Daniel Borkmann wrote:
> Well, BSD has helpers such as bzero_explicit() for such cases to work
> around this, which memzero_explicit() similarly does; see also [1].
>
> [1] https://gcc.gnu.org/ml/gcc-help/2014-10/msg00059.html
We should make memset() a define and call a custom function internally.
Otherwise there are thousands of calls to memset() which we would need
to audit. We could do some of this automatically but it's going to be a
mess.
regards,
dan carpenter
On Mon, Dec 01, 2014 at 02:29:57PM +0300, Dan Carpenter wrote:
> On Mon, Dec 01, 2014 at 12:04:57PM +0100, Daniel Borkmann wrote:
> > Well, BSD has helpers such as bzero_explicit() for such cases to work
> > around this, which memzero_explicit() similarly does; see also [1].
> >
> > [1] https://gcc.gnu.org/ml/gcc-help/2014-10/msg00059.html
>
> We should make memset() a define and call a custom function internally.
>
Or we could specify -fno-builtin-memset like the GCC people suggest.
regards,
dan carpenter
On 12/01/2014 12:29 PM, Dan Carpenter wrote:
> On Mon, Dec 01, 2014 at 12:04:57PM +0100, Daniel Borkmann wrote:
>> Well, BSD has helpers such as bzero_explicit() for such cases to work
>> around this, which memzero_explicit() similarly does; see also [1].
>>
>> [1] https://gcc.gnu.org/ml/gcc-help/2014-10/msg00059.html
>
> We should make memset() a define and call a custom function internally.
>
> Otherwise there are thousands of calls to memset() which we would need
> to audit. We could do some of this automatically but it's going to be a
> mess.
Sort of, yeah; for now random driver and crypto subsystem, which I consider
the main candidates, have been converted and with Julia's latest patch set
from today (via coccinelle) also remaining arch-specific crypto drivers.
On 12/01/2014 12:38 PM, Dan Carpenter wrote:
> On Mon, Dec 01, 2014 at 02:29:57PM +0300, Dan Carpenter wrote:
>> On Mon, Dec 01, 2014 at 12:04:57PM +0100, Daniel Borkmann wrote:
>>> Well, BSD has helpers such as bzero_explicit() for such cases to work
>>> around this, which memzero_explicit() similarly does; see also [1].
>>>
>>> [1] https://gcc.gnu.org/ml/gcc-help/2014-10/msg00059.html
>>
>> We should make memset() a define and call a custom function internally.
>
> Or we could specify -fno-builtin-memset like the GCC people suggest.
If there's a better, reliable way, I'm all for it. To me, memzero_explicit()
seemed like the least intrusive variant doing the job, I'm unsure if globally
setting -fno-builtin-memset will come with unforeseen side-effects on some
archs (besides performance-wise).