From 908da16e8c9758b1e164c49a0d92142f73a3d64c Mon Sep 17 00:00:00 2001 From: matthew <> Date: Fri, 18 Jul 2014 21:40:54 +0000 Subject: [PATCH] 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 --- .../libcrypto/arc4random/arc4random_linux.h | 19 ++++++++----------- src/lib/libcrypto/arc4random/arc4random_osx.h | 19 ++++++++----------- .../libcrypto/arc4random/arc4random_solaris.h | 19 ++++++++----------- src/lib/libcrypto/crypto/arc4random_linux.h | 19 ++++++++----------- src/lib/libcrypto/crypto/arc4random_osx.h | 19 ++++++++----------- src/lib/libcrypto/crypto/arc4random_solaris.h | 19 ++++++++----------- 6 files changed, 48 insertions(+), 66 deletions(-) diff --git a/src/lib/libcrypto/arc4random/arc4random_linux.h b/src/lib/libcrypto/arc4random/arc4random_linux.h index 2319ccbf..f02ae388 100644 --- a/src/lib/libcrypto/arc4random/arc4random_linux.h +++ b/src/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 @@ -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/src/lib/libcrypto/arc4random/arc4random_osx.h b/src/lib/libcrypto/arc4random/arc4random_osx.h index 88433e17..46053a45 100644 --- a/src/lib/libcrypto/arc4random/arc4random_osx.h +++ b/src/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 @@ -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/src/lib/libcrypto/arc4random/arc4random_solaris.h b/src/lib/libcrypto/arc4random/arc4random_solaris.h index ca8e107e..2386dbe8 100644 --- a/src/lib/libcrypto/arc4random/arc4random_solaris.h +++ b/src/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 @@ -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/src/lib/libcrypto/crypto/arc4random_linux.h b/src/lib/libcrypto/crypto/arc4random_linux.h index 2319ccbf..f02ae388 100644 --- a/src/lib/libcrypto/crypto/arc4random_linux.h +++ b/src/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 @@ -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/src/lib/libcrypto/crypto/arc4random_osx.h b/src/lib/libcrypto/crypto/arc4random_osx.h index 88433e17..46053a45 100644 --- a/src/lib/libcrypto/crypto/arc4random_osx.h +++ b/src/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 @@ -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/src/lib/libcrypto/crypto/arc4random_solaris.h b/src/lib/libcrypto/crypto/arc4random_solaris.h index ca8e107e..2386dbe8 100644 --- a/src/lib/libcrypto/crypto/arc4random_solaris.h +++ b/src/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 @@ -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)); } }