[PATCH 0 of 4] Avoid dead store elimination in GCC 11+

[Maxim Dounin]

Hello!

On Tue, Apr 18, 2023 at 02:07:06AM +0300, Vadim Fedorenko via nginx-devel wrote:

> GCC version 11 and newer use more aggressive way to eliminate dead stores
> which ends up removing ngx_memzero() calls in several places. Such optimization
> affects calculations of md5 and sha1 implemented internally in nginx. The
> effect could be easily observed by adding a random data to buffer array in
> md5_init() or sha1_init() functions. With this simple modifications the result
> of the hash computation will be different each time even though the provided
> data to hash is not changed.

If calculations of md5 and sha1 are affected, this means that the 
stores in question are not dead, and they shouldn't be eliminated 
in the first place.  From your description this looks like a bug 
in the compiler in question.

Alternatively, this can be a bug in nginx code which makes the 
compiler think that it can eliminate these ngx_memzero() calls - for 
example, GCC is known to do such things if it sees an undefined 
behaviour in the code.

You may want to elaborate more on how to reproduce this, and, if 
possible, how to build a minimal test case which demonstrates the 
problem.

> Changing the code to use current implementation
> of ngx_explicit_memzero() doesn't help because of link-time optimizations
> enabled in RHEL 9 and derivatives. Glibc 2.34 found in RHEL 9 provides
> explicit_bzero() function which should be used to avoid such optimization.
> ngx_explicit_memzero() is changed to use explicit_bzero() if possible.

The ngx_explicit_memzero() function is to be used when zeroed data 
are indeed not used afterwards, for example, to make sure 
passwords are actually eliminated from memory.  It shouldn't be 
used instead of a real ngx_memzero() call - doing so might hide 
the problem, which is either in the compiler or in nginx, but 
won't fix it.

As for using explicit_bzero() for it, we've looked into various 
OS-specific solutions, though there are too many variants out 
there, so it was decided that having our own simple implementation 
is a better way to go.  If it doesn't work in the particular 
setup, it might make sense to adjust our implementation - but 
given the above, it might be the same issue which causes the 
original problem.

-- 
Maxim Dounin
http://mdounin.ru/