[openssl-dev] [openssl.org #4684] Potential problem with OPENSSL_cleanse

[Andy Polyakov]

>> We do have assembler versions for most CPI's.
> 
> In the context one can also add that the kind of optimization that could
> omit memset invocation *has to* rely on deep inter-procedural
> *multi-file* analysis. If compiler is given mem_clr.c alone, and it
> doesn't look at it when compiling other modules, it won't be able to
> eliminate the call. I said it several times already that no security
> software should encourage deep inter-procedural optimizations such as
> -flto (or -ipo in Intel compiler context).
> 
> For reference code generated by Intel compiler (when it's compiling
> mem_clr.c alone) is equivalent to
> 
> f = memset_func;
> if (f==memset) _intel_fast_memset(args)
> else           (*f)(args)

Actually it should also be noted that snippet presented in originally
mentioned
http://www.metzdowd.com/pipermail/cryptography/2016-September/030151.html
is actually compiles as just

_intel_fast_memset(args)

by Intel compiler 17.0 (a.k.a. 2017). I.e. it actually does *not*
reference *that* pointer to memset!!! Which in turn means that report is
inaccurate claiming that compiler fulfills the contract by always
referencing the pointer [just not calling the procedure]. It doesn't,
and one can probably argue that compiler doesn't comply with the
standard. Or can one? What's going on there? Trouble is that pointer is
also declared as const in that example. But what does "const volatile"
even mean? In physical world it's actually a meaningless combination,
nothing can be both. And even logically it's meaningless. And if so, can
one actually blame compiler for making an impossible choice? In addition
one should not forget that the referred snippet declares "erase" static,
which means that it ought to be a header that gets included, which in
turn means that inter-procedural *multi-file* analysis is not required
to eliminate it.

To summarize. OpenSSL fall-back[!] OPENSSL_cleanse subroutine is
different from snippet presented in report in question in two essential
ways:

- pointer to memset is *not* const (which by the way is not a
coincidence, at least in sense that I would have argued against
declaring it volatile const, and I likely did, just can't actually recall);

- it's in separate file (and hence it takes deep IPA to even nominate it
for optimization, which we shouldn't/don't recommend);

And once again, not to forget that it's a fall-back[!] procedure for
platforms that don't have assembly pack. [Well, with exception for arm64
in 1.0.2. I mean arm64cpuid.S in 1.0.2 doesn't have OPENSSL_cleanse.
Should I back-port one from master? And MIPS. I mean MIPS assembly pack
doesn't have "cpuid" module...]