Re: thoughts on reallocarray, explicit_bzero?

[Brent Cook <busterb@gmail.com>]

On Wed, Jan 28, 2015 at 8:19 PM, Rich Felker <dalias@libc.org> wrote:
> On Wed, Jan 28, 2015 at 11:34:20PM +0100, Daniel Cegiełka wrote:
>> 2015-01-28 23:01 GMT+01:00 Daniel Cegiełka <daniel.cegielka@gmail.com>:
>> > 2014-05-19 18:16 GMT+02:00 Rich Felker <dalias@libc.org>:
>> >> On Mon, May 19, 2014 at 05:44:59PM +0200, Daniel Cegiełka wrote:
>> >
>> >>> diff -urN musl.orig/src/string/explicit_bzero.c musl/src/string/explicit_bzero.c
>> >>> --- musl.orig/src/string/explicit_bzero.c     Thu Jan  1 00:00:00 1970
>> >>> +++ musl/src/string/explicit_bzero.c  Fri May  9 09:57:45 2014
>> >>> @@ -0,0 +1,8 @@
>> >>> +#include <string.h>
>> >>> +
>> >>> +static void *(*volatile explicit_memset)(void *, int, size_t) = memset;
>> >>> +
>> >>> +void explicit_bzero(void *b, size_t len)
>> >>> +{
>> >>> +     (*explicit_memset)(b, 0, len);
>> >>> +}
>> >>
>> >> This is a nice trick, but IIRC I actually observed GCC optimizing out
>> >> similar code before (instead of your static volatile, I used a
>> >> volatile compound literal). At least the concept is right though: you
>> >> want to prevent the compiler from being able to do any flow analysis
>> >> at compile time, and making the function pointer volatile achieves
>> >> this rather well. On the other hand, GCC will put the volatile pointer
>> >> (if it even emits it) in non-constant memory, meaning it's an
>> >> additional attack vector for function-pointer-overwrite attacks.
>> >
>> > Linux kernel has similar functions and uses a barrier() here:
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/lib/string.c?id=refs/tags/v3.19-rc6#n600
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/compiler.h?id=refs/tags/v3.19-rc6#n162
>> >
>> > Is such a solution is more correct (and still portable)?
>>
>> I'm afraid that the only appropriate solution is to use memset_s()
>> from C11 and the expectation that the compiler will accept it.
>> barrier() does not give any guarantee that this function will be
>> secure. Only compiler decides. I'm afraid that OpenBSD goes bad path
>> with explicit_bzero(). The same applies to the linux kernel and
>> memzero_explicit().. very stupid name...
>
> I see no way memset_s is technically "better". It's unable to find and
> clear other temporary copies that have been made, and the barrier
> method described above already reliably clears the pointed-to copy.
>
> Rich

Whatever method you choose, the method of testing is an interesting
one, since seeing if the compiler optimized out a memset (because the
memory was not read after a write) requires tricking the compiler into
believing you aren't reading it. This test is pretty cool, IMO:

https://github.com/libressl-portable/openbsd/blob/master/src/regress/lib/libc/explicit_bzero/explicit_bzero.c

it is described a bit more here:
https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX

Getting around link-time optimizations required building the
explicit_bzero function with independent compiler flags to ensure LTO
was not enabled.