common.cpp: int64_t cluster_seedgen(void) fails to use system entropy source in windows

[terrylyons]

Caffe frequently needs entropy to seed PRGNs. The algorithm it uses to do this is unix specific. If it is not successful (always in windows) it falls back to a simple algebraic approach using the time, pid of the process to generate a seed and logs a warning. This has two bad effects. One the different seeds are far from independent which could be serious in a silent way, and two, output is seriously cluttered with frequent log warnings about lack of a device to produce entropy.

The optimum solution would seem to me to use std::random_device()() which provides a strong platform independent way of generating such entropy using native C++. Certainly this is supported in windows 2013 and works for me.

The first few tests I run with this change look so much more acceptable without all the log messages.

My modifications of common.cpp are included below:

`

if BOOST_NO_CXX11_HDR_RANDOM

else

include

endif

namespace caffe {

// random seeding
int64_t cluster_seedgen(void) {
int64_t s, seed, pid;
FILE* f = fopen("/dev/urandom", "rb");
if (f && fread(&seed, 1, sizeof(seed), f) == sizeof(seed)) {
fclose(f);
return seed;
}

if BOOST_NO_CXX11_HDR_RANDOM

else

std::random_device rd;
if (rd.entropy() >= 32){
unsigned long long seed = rd();
seed <<= 32;
seed += rd();
return seed;
}

endif

LOG(INFO) << "System entropy source not available, "
"using fallback algorithm to generate seed instead.";
if (f)
fclose(f);

pid = getpid();
s = time(NULL);
seed = std::abs(((s * 181) * ((pid - 83) * 359)) % 104729);
return seed;
}

} // namespace caffe
`

[willyd]

I encourage you to submit a PR about this. Please follow the contribution guidelines.

[terrylyons]

Thanks for proposing that I submit this. Unfortunately, at the current time I really don't have time to do this properly. In particular, I have no idea if I really have to test for the CXX11 headers etc. I do no that one does need to leave the old unix approach in - since Linux systems I used recently seem to always return 0 as the amount of entropy available through the C++ standard device. Bizarre but true!

The small piece of code I wrote seems a good fix to a real problem. So if you or someone else want to put it up then I am comfortable with this. When I have time I will come back to it.

[willyd]

Looks like #3306 or using CPython's way with CryptGenRandom might be a better alternative here.

[terrylyons]

The std:: random_device approach seems preferred language invariant one for the long term. Microsoft are on record to guarantee that their implementation is cryptographically secure, g++ Linux is said to use (u)random so either way you get a strong Implimentation.

On 7 Oct 2016, at 17:05, Guillaume Dumont <notifications@github.commailto:notifications@github.com> wrote:

Looks like #3306#3306 or using CPython's wayhttps://github.com/python/cpython/blob/4106e4237df5397efc785ce4daa257aecb5bff75/Python/random.c#L65 with CryptGenRandom might be a better alternative here.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/4293#issuecomment-252290636, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGFm-4DdoEDeumVJePcNewzVBZVh3nJ4ks5qxmv9gaJpZM4Izuz4.

[Licheng314]

Excuse me, I can't tell how did you modificate the code.
What does it means:
if BOOST_NO_CXX11_HDR_RANDOM
else
include
endif
Can you explain it more clearly?

[Licheng314]

I just inserted the code below in line 36:

std::random_device rd;
if (rd.entropy() >= 32){
unsigned long long seed = rd();
seed <<= 32;
seed += rd();
return seed;
}

Then the log message not show again. Did I do it correctly?

[terrylyons]

Random_device is only available since C++11; this variable tests for the header. I was reluctant to change caffe myself since I am not aware of the coding conventions and assumptions made about which versions of C++ etc. can be assumed. Unfortunately, the version of g++ I tested on Ubuntu did not really implement the C++ random_device well and returned an entropy of 0 – which is really irritating and makes a robust implementation slightly more challenging. It would be good to fix this; maybe it already is fixed. In my view caffe should always avoid platform based solutions when there are good cross-platform language conformant solutions. However, as I mention above, code may sometimes need to conditionally fall back to platform based solutions until the platform fully supports the language feature. Best Terry From: Licheng314 [mailto:notifications@github.com] Sent: 28 February 2017 09:19 To: BVLC/caffe <caffe@noreply.github.com> Cc: Terry Lyons <tlyons@maths.ox.ac.uk>; Author <author@noreply.github.com> Subject: Re: [BVLC/caffe] common.cpp: int64_t cluster_seedgen(void) fails to use system entropy source in windows (#4293) Excuse me, I can't tell how did you modificate the code. What does it means: if BOOST_NO_CXX11_HDR_RANDOM else include endif Can you explain it more clearly? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#4293 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGFm-21dbIkF4yKZ034I5GVKY-GLMeXrks5rg-aWgaJpZM4Izuz4>.

[Licheng314]

What ever, I only use caffe in Windows by myself.
I modified the caffe as you told, and used the header: .
Can I just inserted the code below in line 36 to solve the problem "fails to use system entropy source in windows"?

std::random_device rd;
if (rd.entropy() >= 32){
unsigned long long seed = rd();
seed <<= 32;
seed += rd();
return seed;
}