Opened 10 years ago

Closed 5 years ago

#7248 closed Bugs (worksforme)

UUID Conditional jump or move depends on uninitialised value(s)

Reported by: df@… Owned by: Andy Tompkins
Milestone: Boost 1.66.0 Component: uuid
Version: Boost 1.55.0 Severity: Problem
Keywords: uuid, valgrind Cc:

Description

The following code produces Valgrind warnings:

Code:

boost::uuids::uuid uuid = boost::uuids::random_generator()();

Valgrind:

==5381== Memcheck, a memory error detector
==5381== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==5381== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==5381== Command: ../../bin/test/test_r14p
==5381== 
==5381== Conditional jump or move depends on uninitialised value(s)
==5381==    at 0x40296B: main (mersenne_twister.hpp:177)
==5381==  Uninitialised value was created by a heap allocation
==5381==    at 0x4C2747E: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5381==    by 0x4041AD: boost::uuids::detail::seed_rng::sha1_random_digest_() (seed_rng.hpp:162)
==5381==    by 0x402910: main (seed_rng.hpp:103)
==5381== 
==5381== Conditional jump or move depends on uninitialised value(s)
==5381==    at 0x40298D: main (mersenne_twister.hpp:179)
==5381==  Uninitialised value was created by a heap allocation
==5381==    at 0x4C2747E: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5381==    by 0x4041AD: boost::uuids::detail::seed_rng::sha1_random_digest_() (seed_rng.hpp:162)
==5381==    by 0x402910: main (seed_rng.hpp:103)
==5381== 
==5381== Use of uninitialised value of size 8
==5381==    at 0x54DD208: ??? (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54E25AA: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54E27A5: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54F4FCD: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x402CCD: main (ostream:195)
==5381==  Uninitialised value was created by a heap allocation
==5381==    at 0x4C2747E: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5381==    by 0x4041AD: boost::uuids::detail::seed_rng::sha1_random_digest_() (seed_rng.hpp:162)
==5381==    by 0x404BBE: void boost::random::detail::fill_array_int_impl<32, 624ul, boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>, unsigned int>(boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>&, boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>, unsigned int (&) [624ul]) (seed_rng.hpp:103)
==5381==    by 0x40295B: main (seed_impl.hpp:324)
==5381== 
==5381== Conditional jump or move depends on uninitialised value(s)
==5381==    at 0x54DD20E: ??? (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54E25AA: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54E27A5: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x54F4FCD: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib64/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6.0.13)
==5381==    by 0x402CCD: main (ostream:195)
==5381==  Uninitialised value was created by a heap allocation
==5381==    at 0x4C2747E: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5381==    by 0x4041AD: boost::uuids::detail::seed_rng::sha1_random_digest_() (seed_rng.hpp:162)
==5381==    by 0x404BBE: void boost::random::detail::fill_array_int_impl<32, 624ul, boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>, unsigned int>(boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>&, boost::uuids::detail::generator_iterator<boost::uuids::detail::seed_rng>, unsigned int (&) [624ul]) (seed_rng.hpp:103)
==5381==    by 0x40295B: main (seed_impl.hpp:324)
==5381== 
c0dbd170-11c6-4877-9180-d26753748484
==5381== 
==5381== HEAP SUMMARY:
==5381==     in use at exit: 0 bytes in 0 blocks
==5381==   total heap usage: 128 allocs, 128 frees, 3,596 bytes allocated
==5381== 
==5381== All heap blocks were freed -- no leaks are possible
==5381== 
==5381== For counts of detected and suppressed errors, rerun with: -v
==5381== ERROR SUMMARY: 48 errors from 4 contexts (suppressed: 6 from 6)

The following works without any warnings:

boost::mt19937 ran;
boost::uuids::uuid uuid = boost::uuids::random_generator(ran)();

Change History (7)

comment:1 by Andy Tompkins, 10 years ago

I believe I have fixed this with trunk commit #80501.

Can you confirm please.

comment:2 by Andy Tompkins, 9 years ago

Resolution: fixed
Status: newclosed

comment:3 by k.stuhlemmer@…, 8 years ago

Resolution: fixed
Severity: OptimizationShowstopper
Status: closedreopened
Version: Boost 1.48.0Boost Development Trunk

This is a serious issue! Using uninitialized variables (i.e. _rd[]) as extra source of randomness has been proven to be a very bad idea (see http://kqueue.org/blog/2012/06/25/more-randomness-or-less). You should consider to rework the seed generation or remove it completely. Boost is believed to be high-quality code and many developers trust on it. In this certain case its usage is simply dangerous.

comment:4 by anonymous, 8 years ago

I have observed the same issue in boost 1.55.0

Are there any plans to fix this?

Besides creating a lot of noise in my case, I agree that relying on uninitialized values as a source of entropy is simply not safe.

comment:5 by douwegelling@…, 6 years ago

I'd just like to draw some attention to this issue. I'd prefer if uuid::random_generator did not provide a default constructor to the current state where a you can do so, but it takes data from uninitialized variables in lieu of entropy.

comment:6 by James E. King, III, 5 years ago

I'm wondering if we should simply eliminate the current default construction with the sha1 code, and require a random generator from boost::random, such as mt19937 (or perhaps that becomes the default), or random_device.

comment:7 by James E. King, III, 5 years ago

Milestone: To Be DeterminedBoost 1.66.0
Resolution: worksforme
Severity: ShowstopperProblem
Status: reopenedclosed
Version: Boost Development TrunkBoost 1.55.0

This is no longer reproducible. I believe the issue was fixed in boost 1.59 or earlier; see https://svn.boost.org/trac10/ticket/9407. I am resolving this as "worksforme" in boost 1.66 however I suspect it was fixed as far back as 1.59.

Note: See TracTickets for help on using tickets.