explicit_bzero optimized away when using link-time optimization

[jiixyj]

Hi, I've adapted "libressl-2.0.2/tests/explicit_bzero.c" to test some implementations of explicit_bzero. Compiling this test file with "gcc49 -O1 -flto -DELF_HOOK_IMPL gistfile1.c" on FreeBSD optimizes away the explicit_bzero calls and the __explicit_bzero_hook symbol, resulting in a test failure. This is the current implementation used by OpenBSD and LibreSSL. I don't know if this is caused by toolchain advances or differences in weak symbol semantics. On OpenBSD 5.5 the test works fine with the system compiler and GCC 4.8 from ports.

I've fixed the elf hook implementation by removing the default implementation of explicit_bzero_hook and putting the call to explicit_bzero_hook behind an if check.

Another fix could be to use one of the VOLATILE_{1,2,3}_IMPL implementations as they rely on portable C semantics.

[busterb]

Thanks for the report and suggested fix, we'll take a look.

[busterb]

Confirmed, I can reproduce on Ubuntu with gcc 4.9.1 using your test file as well.

[busterb]

I tried to recreate this same test within libressl but could not. I think the test might have a slight flaw.

Note that libressl builds explicit_bzero with independent compiler flags (as part of the 'libcompatnoopt' library, an automake hack to allow using different flags). This prevents the compiler from considering this object file for LTO even if it was enabled globally for the project. I tried compiling the explicit_bzero portion of your test file as a separate object as well. Linking that with the rest of the test, I was unable to reproduce, though I may have done something wrong.

Are you able to reproduce any issues when explicit_bzero is built as an independent object, or able to produce any CFLAGS/LDFLAGS, etc. settings that can cause the same when libressl is building libcompatnoopt?

[jiixyj]

Yes, putting explicit_zero into its own source file and compiling with:

gcc49 -O1 -flto -DELF_HOOK_IMPL -c explicit_bzero.c gcc49 -O1 -flto -DEXTERN_IMPL test.c explicit_bzero.o

also makes the test fail. There has to be a function prototype for explicit_bzero in the test case, though. If there isn't, GCC won't optimize the symbol away and warns with "warning: implicit declaration of function 'explicit_bzero'" when using -Wall. So currently, the unmodified test case won't ever fail on systems that don't have the explicit_zero declaration in string.h (at least when using GCC).

I'll check again if I can reproduce the issue within libcompatnoopt.

[jiixyj]

I can confirm that when '-O0 -flto' is used the symbol is not optimized away. So libressl is safe for now. I guess this still could catch people out who drop explicit_bzero.c from OpenBSD into their own projects.

[bob-beck]

People who drop code from OpenBSD into their own projects without later following updates to it, or paying attention to how it is compiled, or how things are different in openbsd as compared to their own projects, will get caught. We have seen this with other things before. (Such as arc4random in freebsd)

On Thu, Jul 31, 2014 at 7:35 AM, jiixyj notifications@github.com wrote:

I can confirm that when '-O0 -flto' is used the symbol is not optimized away. So libressl is safe for now. I guess this still could catch people out who drop explicit_bzero.c from OpenBSD into their own projects.

— Reply to this email directly or view it on GitHub https://github.com/libressl-portable/openbsd/issues/5#issuecomment-50759010 .

[jiixyj]

I agree completely. I have tested this now on OpenBSD 5.5 with GCC 4.8.2 from ports:

egcc -O1 -flto -c explicit_bzero.c egcc -O1 -flto -fwhole-program -DEXTERN_IMPL test.c explicit_bzero.o

…and the test fails. So "-fwhole-program" is needed to uncover this problem on OpenBSD. With this patch here the test passes again. GCC doesn't seem to remove weak symbols if they are not defined even when doing whole program optimization. I didn't rebuild the whole system with this patch, though.

[ghost]

We might want to add a similar workaround as done by BoringSSL in

https://boringssl.googlesource.com/boringssl/+/ad1907fe73334d6c696c8539646c21b11178f20f%5E!/#F0

and adding this to explicit_bzero():

/* As best as we can tell, this is sufficient to break any optimisations that might try to eliminate "superfluous" memsets. If there's an easy way to detect memset_s, it would be better to use that. */

if defined(OPENSSL_WINDOWS)

**asm;

else

__asm volatile**("" : : "r"(ptr) : "memory");

endif

[busterb]

I will run it by matthew as a general improvement in the openbsd libc. This should be useful at some point, though this isn't an issue how we build it in libressl today.