Re: Dead stores in src/common/sha2.c

Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
> On 29/05/2019 18:47, Hamlin, Garick L wrote:
>> On Wed, May 29, 2019 at 11:01:05AM -0400, Tom Lane wrote:
>>> At the same time, I'm not sure if we should just write this off as an
>>> ignorable warning. If the C compiler concludes these are dead stores
>>> it'll probably optimize them away, leading to not accomplishing the
>>> goal of wiping the values.

>> Yeah, I mean it's odd to put code in to zero/hide state knowing it's
>> probably optimized out.
>> We could also take it out, but maybe it does help somewhere?
>> ... or put in a comment that says: This probably gets optimized away, but
>> we don't consider it much of a risk.

> There's a function called explicit_bzero() in glibc, for this purpose.
> See
> https://www.gnu.org/software/libc/manual/html_node/Erasing-Sensitive-Data.html.
> It's not totally portable, but it's also available in some BSDs, at
> least. That documentation mentions the possibility that it might force
> variables to be stored in memory that would've otherwise been kept only
> in registers, but says that in most situations it's nevertheless better
> to use explicit_bero() than not. So I guess we should use that, if it's
> available.

Meh. After looking at this closer, I'm convinced that doing anything
that might force the variables into memory would be utterly stupid.
Aside from any performance penalty we'd take, that would make their
values more observable not less so.

In any case, though, it's very hard to see why we should care in the
least. Somebody who can observe the contents of server memory (or
application memory, in the case of frontend usage) can surely extract
the input or output of the SHA2 transform, which is going to be way
more useful than a few intermediate values.

So I think we should either do nothing, or suppress this warning by
commenting out the variable-zeroing.

(Note that an eyeball scan finds several more dead zeroings besides
the ones in Garick's patch. Some of them are ifdef'd out ...)

regards, tom lane