There was recently a patch to the random driver to replace memset()
because, according to the submitter, gcc sometimes optimises memset()
away which might leave data unnecessarily exposed. The solution
suggested was a function called memzero_explicit(). There was a fair
bit of discussion and the patch was accepted.
In the crypto directory of the kernel source I have:
$ grep memset *.c | wc -l
133
$
I strongly suspect some of these should be fixed.
Hi Sandy,
On 10/05/2014 05:09 AM, Sandy Harris wrote:
> There was recently a patch to the random driver to replace memset()
> because, according to the submitter, gcc sometimes optimises memset()
> away which might leave data unnecessarily exposed. The solution
> suggested was a function called memzero_explicit(). There was a fair
> bit of discussion and the patch was accepted.
>
> In the crypto directory of the kernel source I have:
>
> $ grep memset *.c | wc -l
> 133
> $
>
> I strongly suspect some of these should be fixed.
I have submitted it here one month ago for crypto and it's still
awaiting to be applied:
http://www.spinics.net/lists/linux-crypto/msg11965.html
As the random driver patch has been applied to random -dev, it will be
available from 3.18 onwards, but the dependency for crypto is currently
there, that's why I asked Ted to take it through his tree; hopefully
this will happen soonish (but I haven't heard anything back ever since) ...
Thanks!
Daniel
On Sat, Oct 04, 2014 at 11:09:40PM -0400, Sandy Harris wrote:
> There was recently a patch to the random driver to replace memset()
> because, according to the submitter, gcc sometimes optimises memset()
> away which might leave data unnecessarily exposed. The solution
> suggested was a function called memzero_explicit(). There was a fair
> bit of discussion and the patch was accepted.
Do you have a pointer?
> In the crypto directory of the kernel source I have:
>
> $ grep memset *.c | wc -l
> 133
> $
>
> I strongly suspect some of these should be fixed.
It seems this is a common topic:
http://www.daemonology.net/blog/2014-09-04-how-to-zero-a-buffer.html
and part 2:
http://www.daemonology.net/blog/2014-09-06-zeroing-buffers-is-insufficient.html
Summary: It's not just the memory we should be concerned about, there's also
the stack and the registers.
memzero_explicit() is a good start, but I think the articles raise two
important points. a) register data temporarily stored on the stack via
compiler optimizations, and b) aesni register contents left behind after
use.
I suspect (a) is an unquantified problem on embedded systems, and (b)
may extend to embedded hardware crypto implementations.
thx,
Jason.
On Mon, Oct 6, 2014 at 1:44 PM, Jason Cooper <[email protected]> wrote:
> On Sat, Oct 04, 2014 at 11:09:40PM -0400, Sandy Harris wrote:
>> There was recently a patch to the random driver to replace memset()
>> because, according to the submitter, gcc sometimes optimises memset()
>> away which might leave data unnecessarily exposed. The solution
>> suggested was a function called memzero_explicit(). There was a fair
>> bit of discussion and the patch was accepted.
>
> Do you have a pointer?
https://lkml.org/lkml/2014/8/25/497
On Mon, Oct 06, 2014 at 01:59:06PM -0400, Sandy Harris wrote:
> On Mon, Oct 6, 2014 at 1:44 PM, Jason Cooper <[email protected]> wrote:
>
> > On Sat, Oct 04, 2014 at 11:09:40PM -0400, Sandy Harris wrote:
> >> There was recently a patch to the random driver to replace memset()
> >> because, according to the submitter, gcc sometimes optimises memset()
> >> away which might leave data unnecessarily exposed. The solution
> >> suggested was a function called memzero_explicit(). There was a fair
> >> bit of discussion and the patch was accepted.
> >
> > Do you have a pointer?
>
> https://lkml.org/lkml/2014/8/25/497
Ok, that was the same thread I found. I was looking for the 'fair bit
of discussion' part. ;-)
thx,
Jason.
On Mon, Oct 6, 2014 at 1:44 PM, Jason Cooper <[email protected]> wrote:
> On Sat, Oct 04, 2014 at 11:09:40PM -0400, Sandy Harris wrote:
>> There was recently a patch to the random driver to replace memset()
>> because, according to the submitter, gcc sometimes optimises memset()
>> away ...
> memzero_explicit() is a good start, ...
As I see it, memzero_explicit() is a rather ugly kluge, albeit an
acceptable one in the circumstances.
A real fix would make memset() do the right thing reliably; if the
programmer puts in memset( x, 0, nbytes) then the memory should be
cleared, no ifs or buts. I do not know or care if that means changes
in the compiler or in the library code or even both, but the fix
should make the standard library code work right, not require adding a
new function and expecting everyone to use it.
On 10/06/2014 08:52 PM, Sandy Harris wrote:
> On Mon, Oct 6, 2014 at 1:44 PM, Jason Cooper <[email protected]> wrote:
>> On Sat, Oct 04, 2014 at 11:09:40PM -0400, Sandy Harris wrote:
...
>>> There was recently a patch to the random driver to replace memset()
>>> because, according to the submitter, gcc sometimes optimises memset()
>>> away ...
>
>> memzero_explicit() is a good start, ...
>
> As I see it, memzero_explicit() is a rather ugly kluge, albeit an
> acceptable one in the circumstances.
Right.
> A real fix would make memset() do the right thing reliably; if the
> programmer puts in memset( x, 0, nbytes) then the memory should be
> cleared, no ifs or buts. I do not know or care if that means changes
> in the compiler or in the library code or even both, but the fix
> should make the standard library code work right, not require adding a
> new function and expecting everyone to use it.
That would be a desirable goal, ideally perhaps as a built-in from
the compiler itself, just as memset(). Applications such as openssh
implement for the very same purpose their bzero_explicit() variant
just as well.
I have started a thread about this on the gcc help mailing list
https://gcc.gnu.org/ml/gcc-help/2014-10/msg00047.html
We might consider replacinging memzero_explicit with memset_s() since
that is in the C!! standard, albeit I think as optional. IBM, Apple,
NetBSD, ... have that.
https://mail-index.netbsd.org/tech-userlevel/2012/02/24/msg006125.html
On 10/08/2014 04:30 AM, Sandy Harris wrote:
> I have started a thread about this on the gcc help mailing list
> https://gcc.gnu.org/ml/gcc-help/2014-10/msg00047.html
Great, perhaps you want to pass a patch proposal to gcc folks?
> We might consider replacinging memzero_explicit with memset_s() since
> that is in the C!! standard, albeit I think as optional. IBM, Apple,
> NetBSD, ... have that.
> https://mail-index.netbsd.org/tech-userlevel/2012/02/24/msg006125.html
The patch you point to for NetBSD does nothing else what memzero_explicit()
or bzero_explicit() variants already internally do, only that they're
discussing about whether to adopt it or not in their user space libc ...
Cheers,
Daniel