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.
Index: 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
--- include/thread_private.h 16 Oct 2011 06:29:56 -0000 1.25
+++ include/thread_private.h 20 Sep 2012 22:10:49 -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: stdlib/random.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.17
diff -u -p -r1.17 random.c
--- stdlib/random.c 1 Jun 2012 01:01:57 -0000 1.17
+++ stdlib/random.c 20 Sep 2012 22:10:50 -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: 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
--- thread/unithread_malloc_lock.c 13 Jun 2008 21:18:43 -0000 1.8
+++ thread/unithread_malloc_lock.c 20 Sep 2012 22:10:50 -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;
}
