From e54c0f45bd3dbf23ff668862702dcc414b63635f 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 --- lib/libcrypto/arc4random/arc4random_linux.h | 19 ++++++++----------- lib/libcrypto/arc4random/arc4random_osx.h | 19 ++++++++----------- lib/libcrypto/arc4random/arc4random_solaris.h | 19 ++++++++----------- lib/libcrypto/crypto/arc4random_linux.h | 19 ++++++++----------- lib/libcrypto/crypto/arc4random_osx.h | 19 ++++++++----------- lib/libcrypto/crypto/arc4random_solaris.h | 19 ++++++++----------- 6 files changed, 48 insertions(+), 66 deletions(-) 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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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)); } } -- 2.20.1