| Message ID | CACXcFmkO0g2YRjvfknKXr_ZnJaMg2cpvOsLq=h1ZcB=hg9NK8w@mail.gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | memset() in crypto code | expand |
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
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.
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: