summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlex Dowad <alexinbeijing@gmail.com>2020-05-27 06:01:22 +0200
committerAlex Dowad <alexinbeijing@gmail.com>2020-06-23 16:10:54 +0200
commit7a9f0cc3d05f860268e3fd28fff17d12c5671f97 (patch)
tree042cc8880c1eb9c6e485067582859db9b8b1d217
parent9bd648ba1e54c1f3dc8de19772a4f038fe9320a7 (diff)
downloadphp-git-7a9f0cc3d05f860268e3fd28fff17d12c5671f97.tar.gz
Simplify `_crypt_extended_init_r`, and fix redundant initialization on Win32/Solaris
Looking at the history of this function, the original implementation had a bug where it would return from the middle of the function without unlocking the mutex first. The author attempted to fix this by incrementing the `initialized` flag atomically, which is not necessary, since the section which modifies the flag is protected by a mutex. Coincidentally, at the same time that all this unnecessary 'atomic' machinery was introduced, the code was also changed so that it didn't return without unlocking the mutex. So it looks like the bug was fixed by accident. It's not necessary to declare the flag as `volatile` either, since it is protected by a mutex. Further, the 'fixed' implementation was also wrong in another respect: on Windows and Solaris, the `initialized` flag was not even declared as `static`!! So the initialization of the static tables for S-boxes, P-boxes, etc. was repeated on each call to `php_crypt`, completely defeating the purpose of this function.
-rw-r--r--configure.ac7
-rw-r--r--ext/standard/config.m45
-rw-r--r--ext/standard/php_crypt_r.c23
3 files changed, 3 insertions, 32 deletions
diff --git a/configure.ac b/configure.ac
index 99e04e6c1d..df2c205f19 100644
--- a/configure.ac
+++ b/configure.ac
@@ -643,13 +643,6 @@ if test "$ac_cv_func_getaddrinfo" = yes; then
AC_DEFINE(HAVE_GETADDRINFO,1,[Define if you have the getaddrinfo function])
fi
-dnl Check for the __sync_fetch_and_add builtin.
-AC_CACHE_CHECK([for __sync_fetch_and_add], ac_cv_func_sync_fetch_and_add,
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[int x;__sync_fetch_and_add(&x,1);]])],[ac_cv_func_sync_fetch_and_add=yes],[ac_cv_func_sync_fetch_and_add=no])])
-if test "$ac_cv_func_sync_fetch_and_add" = yes; then
- AC_DEFINE(HAVE_SYNC_FETCH_AND_ADD,1,[Define if you have the __sync_fetch_and_add function])
-fi
-
AC_REPLACE_FUNCS(strlcat strlcpy explicit_bzero getopt)
AC_FUNC_ALLOCA
PHP_TIME_R_TYPE
diff --git a/ext/standard/config.m4 b/ext/standard/config.m4
index 171200d18d..9f82ab1afa 100644
--- a/ext/standard/config.m4
+++ b/ext/standard/config.m4
@@ -389,11 +389,6 @@ if test "$ac_cv_strptime_decl_fails" = "yes"; then
fi
dnl
-dnl Check for atomic operation API availability in Solaris
-dnl
-AC_CHECK_HEADERS([atomic.h])
-
-dnl
dnl Check for arc4random on BSD systems
dnl
AC_CHECK_DECLS([arc4random_buf])
diff --git a/ext/standard/php_crypt_r.c b/ext/standard/php_crypt_r.c
index 96e5905e5a..432657cf47 100644
--- a/ext/standard/php_crypt_r.c
+++ b/ext/standard/php_crypt_r.c
@@ -39,11 +39,6 @@
# include <Wincrypt.h>
#endif
-#ifdef HAVE_ATOMIC_H /* Solaris 10 defines atomic API within */
-# include <atomic.h>
-#else
-# include <signal.h>
-#endif
#include "php_crypt_r.h"
#include "crypt_freesec.h"
@@ -76,29 +71,17 @@ void php_shutdown_crypt_r()
void _crypt_extended_init_r(void)
{
-#ifdef PHP_WIN32
- LONG volatile initialized = 0;
-#elif defined(HAVE_ATOMIC_H) /* Solaris 10 defines atomic API within */
- volatile unsigned int initialized = 0;
-#else
- static volatile sig_atomic_t initialized = 0;
-#endif
+ static int initialized = 0;
#ifdef ZTS
tsrm_mutex_lock(php_crypt_extended_init_lock);
#endif
if (!initialized) {
-#ifdef PHP_WIN32
- InterlockedIncrement(&initialized);
-#elif defined(HAVE_SYNC_FETCH_AND_ADD)
- __sync_fetch_and_add(&initialized, 1);
-#elif defined(HAVE_ATOMIC_H) /* Solaris 10 defines atomic API within */
- membar_producer();
- atomic_add_int(&initialized, 1);
-#endif
+ initialized = 1;
_crypt_extended_init();
}
+
#ifdef ZTS
tsrm_mutex_unlock(php_crypt_extended_init_lock);
#endif