[PATCH 0/8] memset() in crypto code

[PATCH 0/8] memset() in crypto code

[Sandy Harris]

Fairly often we want to clear some memory in crypto code; it holds
things we are done using and do not want to leave lying around where
an enemy might discover them. Typical examples are crypto keys or
random numbers we have generated and used for output.

The obvious way to do this is with memset(address,0,bytes) but there
is a problem with that; because we are done using that memory, the
compiler may optimise away the "useless" memset() call. Using
memzero_explicit(address,bytes) instead solves the problem; that
function is designed to resist the optimisation.

There are well over 100 memset() calls in .c files in the crypto and
security directories. I looked at them all and found about a dozen in
eight files that I thought should be changed to memzero_explicit().
Here they are as patches 1 to 8 in this series.

I did read some code & think moderately carefully, but I do not know
the code deeply & it is possible I have made some errors. I think
false positives (making unnecessary changes) are more likely than
false negatives (not catching necessary changes).

Re: [PATCH 0/8] memset() in crypto code

[Christophe Leroy]

Le 16/11/2021 à 12:20, Sandy Harris a écrit :
> Fairly often we want to clear some memory in crypto code; it holds
> things we are done using and do not want to leave lying around where
> an enemy might discover them. Typical examples are crypto keys or
> random numbers we have generated and used for output.
> 
> The obvious way to do this is with memset(address,0,bytes) but there
> is a problem with that; because we are done using that memory, the
> compiler may optimise away the "useless" memset() call. Using
> memzero_explicit(address,bytes) instead solves the problem; that
> function is designed to resist the optimisation.
> 
> There are well over 100 memset() calls in .c files in the crypto and
> security directories. I looked at them all and found about a dozen in
> eight files that I thought should be changed to memzero_explicit().
> Here they are as patches 1 to 8 in this series.
> 
> I did read some code & think moderately carefully, but I do not know
> the code deeply & it is possible I have made some errors. I think
> false positives (making unnecessary changes) are more likely than
> false negatives (not catching necessary changes).
> 

I see no point in doing 8 separate patches that all have the same 
subject and the exact same light description.

I think it would be better to have a single patch with all the changes, 
and use the cover letter description as description for that patch.

Christophe

Re: [PATCH 0/8] memset() in crypto code

[Sandy Harris]

Christophe Leroy <christophe.leroy@csgroup.eu> wrote:

> I see no point in doing 8 separate patches that all have the same
> subject and the exact same light description.
>
> I think it would be better to have a single patch with all the changes,
> and use the cover letter description as description for that patch.

It seemed better to me to have separate patches because there
are 8 files involved, possibly each with a different maintainer.

Likely I should have gone further & included the filenames in
the subject: lines & maintainer addresses in cc:

Re: [PATCH 0/8] memset() in crypto code

[Ard Biesheuvel]

On Tue, 16 Nov 2021 at 12:20, Sandy Harris <sandyinchina@gmail.com> wrote:
>
> Fairly often we want to clear some memory in crypto code; it holds
> things we are done using and do not want to leave lying around where
> an enemy might discover them. Typical examples are crypto keys or
> random numbers we have generated and used for output.
>
> The obvious way to do this is with memset(address,0,bytes) but there
> is a problem with that; because we are done using that memory, the
> compiler may optimise away the "useless" memset() call. Using
> memzero_explicit(address,bytes) instead solves the problem; that
> function is designed to resist the optimisation.
>
> There are well over 100 memset() calls in .c files in the crypto and
> security directories. I looked at them all and found about a dozen in
> eight files that I thought should be changed to memzero_explicit().
> Here they are as patches 1 to 8 in this series.
>
> I did read some code & think moderately carefully, but I do not know
> the code deeply & it is possible I have made some errors. I think
> false positives (making unnecessary changes) are more likely than
> false negatives (not catching necessary changes).

Hello Sandy,

As Greg alluded in reply to one of these patches, memzero_explicit()
is only usually needed for stack variables, because in those cases,
the compiler is able to infer that the memset() is the last thing that
touches the variable before it goes out of scope, and so memset()ing
it can be omitted.

Variables that are passed into a function by pointer reference have a
life time that is not known to the callee, and so there is no way the
compiler can elide memset() calls, which means that using
memzero_explicit() in such cases is not needed. The exception is
functions with static linkage that may end up being inlined into their
callers, but in the crypto subsystem, many such functions are invoked
indirectly via exported function pointers, which makes inlining
impossible.