On Fri, Sep 21, 2012 at 10:36 AM, Alexey Suslikov
<alexey.susli...@gmail.com> wrote:
> On Wed, Sep 19, 2012 at 10:24 PM, Ted Unangst <t...@tedunangst.com> wrote:
>> On Wed, Sep 19, 2012 at 18:50, Alexey Suslikov wrote:
>>> On Wednesday, September 19, 2012, Theo de Raadt wrote:
>>>
>>>> > arc4random() is also thread-safe (it has interal locking) and very
>>>> > desirable for other reasons. But no way to save state.
>>>>
>>>> The last part of this is intentional. Saving the state of pseudo
>>>> random number generators is a stupid concept from the 80's.
>>>>
>>>
>>> I see many rng functions behaving very differently. Is it a good idea
>>> to create a common locking layer on top of need-to-be-safe rng
>>> functions? Or we should deal only with original problem (and only
>>> port random.c code from netbsd)?
>>
>> just slap a mutex around it.
>
> With the diff below Kannel no longer crashes. Only protecting random()
> for now.
>
> Make random() thread-safe by surrounding real call with a mutex locking.
> Found by and diff from Roman Kravchuk. Mainly from NetBSD.
Sorry. Here is correct diff.
We kinda unsure about the approach. For now, we follow arc4random pattern.
Should we use generic _thread_mutex_lock/_thread_mutex_unlock instead?
Index: lib/libc/include/thread_private.h
===================================================================
RCS file: /cvs/src/lib/libc/include/thread_private.h,v
retrieving revision 1.25
diff -u -p -r1.25 thread_private.h
--- lib/libc/include/thread_private.h 16 Oct 2011 06:29:56 -0000 1.25
+++ lib/libc/include/thread_private.h 21 Sep 2012 07:59:34 -0000
@@ -172,4 +172,16 @@ void _thread_arc4_unlock(void);
_thread_arc4_unlock();\
} while (0)
+void _thread_random_lock(void);
+void _thread_random_unlock(void);
+
+#define _RANDOM_LOCK() do { \
+ if (__isthreaded) \
+ _thread_random_lock(); \
+ } while (0)
+#define _RANDOM_UNLOCK() do { \
+ if (__isthreaded) \
+ _thread_random_unlock();\
+ } while (0)
+
#endif /* _THREAD_PRIVATE_H_ */
Index: lib/libc/stdlib/random.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.17
diff -u -p -r1.17 random.c
--- lib/libc/stdlib/random.c 1 Jun 2012 01:01:57 -0000 1.17
+++ lib/libc/stdlib/random.c 21 Sep 2012 07:59:35 -0000
@@ -35,6 +35,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
+#include "thread_private.h"
/*
* random.c:
@@ -376,21 +377,38 @@ setstate(char *arg_state)
*
* Returns a 31-bit random number.
*/
-long
-random(void)
+static long
+random_unlocked(void)
{
int32_t i;
+ int32_t *f, *r;
if (rand_type == TYPE_0)
i = state[0] = (state[0] * 1103515245 + 12345) & 0x7fffffff;
else {
- *fptr += *rptr;
- i = (*fptr >> 1) & 0x7fffffff; /* chucking least random bit */
- if (++fptr >= end_ptr) {
- fptr = state;
- ++rptr;
- } else if (++rptr >= end_ptr)
- rptr = state;
+ /*
+ * Use local variables rather than static variables for speed.
+ */
+ f = fptr; r = rptr;
+ *f += *r;
+ i = (*f >> 1) & 0x7fffffff; /* chucking least random bit */
+ if (++f >= end_ptr) {
+ f = state;
+ ++r;
+ } else if (++r >= end_ptr)
+ r = state;
+ fptr = f; rptr = r;
}
return((long)i);
+}
+
+long
+random(void)
+{
+ long r;
+
+ _RANDOM_LOCK();
+ r = random_unlocked();
+ _RANDOM_UNLOCK();
+ return (r);
}
Index: lib/libc/thread/unithread_malloc_lock.c
===================================================================
RCS file: /cvs/src/lib/libc/thread/unithread_malloc_lock.c,v
retrieving revision 1.8
diff -u -p -r1.8 unithread_malloc_lock.c
--- lib/libc/thread/unithread_malloc_lock.c 13 Jun 2008 21:18:43 -0000
1.8
+++ lib/libc/thread/unithread_malloc_lock.c 21 Sep 2012 07:59:35 -0000
@@ -21,6 +21,12 @@ WEAK_PROTOTYPE(_thread_arc4_unlock);
WEAK_ALIAS(_thread_arc4_lock);
WEAK_ALIAS(_thread_arc4_unlock);
+WEAK_PROTOTYPE(_thread_random_lock);
+WEAK_PROTOTYPE(_thread_random_unlock);
+
+WEAK_ALIAS(_thread_random_lock);
+WEAK_ALIAS(_thread_random_unlock);
+
void
WEAK_NAME(_thread_malloc_lock)(void)
{
@@ -53,6 +59,18 @@ WEAK_NAME(_thread_arc4_lock)(void)
void
WEAK_NAME(_thread_arc4_unlock)(void)
+{
+ return;
+}
+
+void
+WEAK_NAME(_thread_random_lock)(void)
+{
+ return;
+}
+
+void
+WEAK_NAME(_thread_random_unlock)(void)
{
return;
}
Index: lib/librthread/rthread.c
===================================================================
RCS file: /cvs/src/lib/librthread/rthread.c,v
retrieving revision 1.66
diff -u -p -r1.66 rthread.c
--- lib/librthread/rthread.c 22 Aug 2012 23:43:32 -0000 1.66
+++ lib/librthread/rthread.c 21 Sep 2012 07:59:36 -0000
@@ -674,6 +674,8 @@ static void *__libc_overrides[] __used =
&__errno,
&_thread_arc4_lock,
&_thread_arc4_unlock,
+ &_thread_random_lock,
+ &_thread_random_unlock,
&_thread_atexit_lock,
&_thread_atexit_unlock,
&_thread_malloc_lock,
Index: lib/librthread/rthread_fork.c
===================================================================
RCS file: /cvs/src/lib/librthread/rthread_fork.c,v
retrieving revision 1.6
diff -u -p -r1.6 rthread_fork.c
--- lib/librthread/rthread_fork.c 22 Aug 2012 23:43:32 -0000 1.6
+++ lib/librthread/rthread_fork.c 21 Sep 2012 07:59:36 -0000
@@ -91,6 +91,7 @@ _dofork(int is_vfork)
_thread_atexit_lock();
_thread_malloc_lock();
_thread_arc4_lock();
+ _thread_random_lock();
#if defined(__ELF__)
if (_DYNAMIC)
@@ -107,6 +108,7 @@ _dofork(int is_vfork)
_thread_arc4_unlock();
_thread_malloc_unlock();
_thread_atexit_unlock();
+ _thread_random_unlock();
#if defined(__ELF__)
if (_DYNAMIC)
Index: lib/librthread/rthread_libc.c
===================================================================
RCS file: /cvs/src/lib/librthread/rthread_libc.c,v
retrieving revision 1.10
diff -u -p -r1.10 rthread_libc.c
--- lib/librthread/rthread_libc.c 17 Apr 2012 15:10:11 -0000 1.10
+++ lib/librthread/rthread_libc.c 21 Sep 2012 07:59:36 -0000
@@ -200,3 +200,19 @@ _thread_arc4_unlock(void)
_spinunlock(&arc4_lock);
}
+/*
+ * random lock
+ */
+static _spinlock_lock_t random_lock = _SPINLOCK_UNLOCKED;
+
+void
+_thread_random_lock(void)
+{
+ _spinlock(&random_lock);
+}
+
+void
+_thread_random_unlock(void)
+{
+ _spinunlock(&random_lock);
+}
