From: matthew <matthew@openbsd.org>
Date: Fri, 18 Jul 2014 21:40:54 +0000 (+0000)
Subject: Cleanup portable arc4random fork detection code:
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e54c0f45bd3dbf23ff668862702dcc414b63635f;p=openbsd

Cleanup portable arc4random fork detection code:

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
---

diff --git a/lib/libcrypto/arc4random/arc4random_linux.h b/lib/libcrypto/arc4random/arc4random_linux.h
index 2319ccbf427..f02ae388d5f 100644
--- a/lib/libcrypto/arc4random/arc4random_linux.h
+++ b/lib/libcrypto/arc4random/arc4random_linux.h
@@ -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));
 	}
 }
 
diff --git a/lib/libcrypto/arc4random/arc4random_osx.h b/lib/libcrypto/arc4random/arc4random_osx.h
index 88433e17ddb..46053a45b9f 100644
--- a/lib/libcrypto/arc4random/arc4random_osx.h
+++ b/lib/libcrypto/arc4random/arc4random_osx.h
@@ -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));
 	}
 }
 
diff --git a/lib/libcrypto/arc4random/arc4random_solaris.h b/lib/libcrypto/arc4random/arc4random_solaris.h
index ca8e107e400..2386dbe8851 100644
--- a/lib/libcrypto/arc4random/arc4random_solaris.h
+++ b/lib/libcrypto/arc4random/arc4random_solaris.h
@@ -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));
 	}
 }
 
diff --git a/lib/libcrypto/crypto/arc4random_linux.h b/lib/libcrypto/crypto/arc4random_linux.h
index 2319ccbf427..f02ae388d5f 100644
--- a/lib/libcrypto/crypto/arc4random_linux.h
+++ b/lib/libcrypto/crypto/arc4random_linux.h
@@ -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));
 	}
 }
 
diff --git a/lib/libcrypto/crypto/arc4random_osx.h b/lib/libcrypto/crypto/arc4random_osx.h
index 88433e17ddb..46053a45b9f 100644
--- a/lib/libcrypto/crypto/arc4random_osx.h
+++ b/lib/libcrypto/crypto/arc4random_osx.h
@@ -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));
 	}
 }
 
diff --git a/lib/libcrypto/crypto/arc4random_solaris.h b/lib/libcrypto/crypto/arc4random_solaris.h
index ca8e107e400..2386dbe8851 100644
--- a/lib/libcrypto/crypto/arc4random_solaris.h
+++ b/lib/libcrypto/crypto/arc4random_solaris.h
@@ -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));
 	}
 }