s4-messaging: Use generate_random() to get a unique ID for messaging clients
idra at samba.org
Wed Apr 25 06:59:02 MDT 2012
On Wed, 2012-04-25 at 22:51 +1000, Andrew Bartlett wrote:
> On Wed, 2012-04-25 at 08:04 -0400, simo wrote:
> > On Wed, 2012-04-25 at 11:44 +0200, Andrew Bartlett wrote:
> > > commit b8055132b1c62dd19981fea2822ab9e1829a8ded
> > > Author: Andrew Bartlett <abartlet at samba.org>
> > > Date: Wed Apr 25 17:53:18 2012 +1000
> > >
> > > s4-messaging: Use generate_random() to get a unique ID for
> > > messaging clients
> > >
> > > The call to random() resulted in duplicate values for s3fs
> > > configurations
> > > which, due to the forked child, all started with the same random
> > > seed.
> > >
> > > A future improvement would be to move to a proven unique value.
> > >
> > > Andrew Bartlett
> > >
> > > Autobuild-User: Andrew Bartlett <abartlet at samba.org>
> > > Autobuild-Date: Wed Apr 25 11:43:40 CEST 2012 on sn-devel-104
> > Andrew are you sure you need to use /dev/urandom here ? It doesn't look
> > like you need absolutelu unpredictable numbers here, just non-colliding
> > numbers.
> > You changed the code to draw from urandom, and if it is used often it
> > mean it will suck a lot of entropy out of the system, causing any
> > application that need to use /dev/random to halt.
> > Wouldn't it have been simpler to just run srand(time(NULL)*pid) to get a
> > new seed for the process ?
> You are correct to think carefully about our over-use of random entropy.
> In this case, it isn't used often (typically once per child process in
> s3fs configurations only), and we have logic in generate_random_buffer()
> to attempt to reduce the load on /dev/urandom. We set up our own PRNG,
> where a process has read more than 40 bytes, which I added with
> 6554433fc227baab93398576db703c91db1541f2 back in 2007.
This doesn't really help at process startup, as we reset the global
variable after fork, so for the first 40 bytes at least we do
use /dev/urandom, that's why I had a concern.
> I also worked on a solution using IDR trees to allocate a server_id
> number (along side the pid, for the auth_samba4 use case):
> https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/s3-task-id-allocator however I haven't yet made it work. If I can make it work, I would prefer to use an allocator, for more predictable behaviour under test.
> Another idea I had was to use the combination of pid and memory address
> of the event context or messaging context, but currently 'task' is just
> 32 bits.
> Metze has also suggested some improvements on which fields to randomize
> in https://bugzilla.samba.org/show_bug.cgi?id=8872#c4
Ok, seem like iot is staying on the radar, that's good enough.
> As to if we should use random() and srand(): I would rather not be
> reseeding the srand() as it may be deliberately set by smbtorture
> callers wishing to create reproducible runs, and the messaging client is
> used by smbtorture.
True, although libraries could change it, so depending on nothing ever
calling srand() in smbtorture seem a bit fragile.
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>
More information about the samba-technical