Cleanup portable arc4random fork detection code:
authormatthew <matthew@openbsd.org>
Fri, 18 Jul 2014 21:40:54 +0000 (21:40 +0000)
committermatthew <matthew@openbsd.org>
Fri, 18 Jul 2014 21:40:54 +0000 (21:40 +0000)
1. Use "len" parameter instead of sizeof(*rs).

2. Simplify the atfork handler to be strictly async signal safe by
simply writing to a global volatile sig_atomic_t object, and then
checking for this in _rs_forkdetect().  (Idea from discussions with
Szabolcs Nagy and Rich Felker.)

3. Use memset(rs, 0, sizeof(*rs)) to match OpenBSD's MAP_INHERIT_ZERO
fork semantics to avoid any skew in behavior across platforms.

ok deraadt

lib/libcrypto/arc4random/arc4random_linux.h
lib/libcrypto/arc4random/arc4random_osx.h
lib/libcrypto/arc4random/arc4random_solaris.h
lib/libcrypto/crypto/arc4random_linux.h
lib/libcrypto/crypto/arc4random_osx.h
lib/libcrypto/crypto/arc4random_solaris.h

index 2319ccb..f02ae38 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_linux.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $    */
+/*     $OpenBSD: arc4random_linux.h,v 1.2 2014/07/18 21:40:54 matthew Exp $    */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }
 
index 88433e1..46053a4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_osx.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $      */
+/*     $OpenBSD: arc4random_osx.h,v 1.2 2014/07/18 21:40:54 matthew Exp $      */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }
 
index ca8e107..2386dbe 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_solaris.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $  */
+/*     $OpenBSD: arc4random_solaris.h,v 1.2 2014/07/18 21:40:54 matthew Exp $  */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }
 
index 2319ccb..f02ae38 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_linux.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $    */
+/*     $OpenBSD: arc4random_linux.h,v 1.2 2014/07/18 21:40:54 matthew Exp $    */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }
 
index 88433e1..46053a4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_osx.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $      */
+/*     $OpenBSD: arc4random_osx.h,v 1.2 2014/07/18 21:40:54 matthew Exp $      */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }
 
index ca8e107..2386dbe 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: arc4random_solaris.h,v 1.1 2014/07/18 02:05:55 deraadt Exp $  */
+/*     $OpenBSD: arc4random_solaris.h,v 1.2 2014/07/18 21:40:54 matthew Exp $  */
 
 /*
  * Copyright (c) 1996, David Mazieres <dm@uun.org>
@@ -27,21 +27,18 @@ _rs_allocate(size_t len)
 {
        void *p;
 
-       if ((p = mmap(NULL, sizeof(*rs), PROT_READ|PROT_WRITE,
+       if ((p = mmap(NULL, len, PROT_READ|PROT_WRITE,
            MAP_ANON|MAP_PRIVATE, -1, 0)) == MAP_FAILED)
                return (NULL);
        return (p);
 }
 
+static volatile sig_atomic_t _rs_forked;
+
 static inline void
 _rs_forkhandler(void)
 {
-       /*
-        * Race-free because we're running single-threaded in a new
-        * address space, and once allocated rs is never deallocated.
-        */
-       if (rs)
-               rs->rs_count = 0;
+       _rs_forked = 1;
 }
 
 static inline void
@@ -50,11 +47,11 @@ _rs_forkdetect(void)
        static pid_t _rs_pid = 0;
        pid_t pid = getpid();
 
-       /* If a system lacks MAP_INHERIT_ZERO, resort to getpid() */
-       if (_rs_pid == 0 || _rs_pid != pid) {
+       if (_rs_pid == 0 || _rs_pid != pid || _rs_forked) {
                _rs_pid = pid;
+               _rs_forked = 0;
                if (rs)
-                       rs->rs_count = 0;
+                       memset(rs, 0, sizeof(*rs));
        }
 }