Conceptually, this is a squashed version of:
Revert "Address feedback"
This reverts commit 75551e07bd.
and
Revert "Add CRYPTO_thread_glock_new"
This reverts commit ed6b2c7938.
But there were some intervening commits that made neither revert apply
cleanly, so instead do it all as one shot.
The crypto global locks were an attempt to cope with the awkward
POSIX semantics for pthread_atfork(); its documentation (the "RATIONALE"
section) indicates that the expected usage is to have the prefork handler
lock all "global" locks, and the parent and child handlers release those
locks, to ensure that forking happens with a consistent (lock) state.
However, the set of functions available in the child process is limited
to async-signal-safe functions, and pthread_mutex_unlock() is not on
the list of async-signal-safe functions! The only synchronization
primitives that are async-signal-safe are the semaphore primitives,
which are not really appropriate for general-purpose usage.
However, the state consistency problem that the global locks were
attempting to solve is not actually a serious problem, particularly for
OpenSSL. That is, we can consider four cases of forking application
that might use OpenSSL:
(1) Single-threaded, does not call into OpenSSL in the child (e.g.,
the child calls exec() immediately)
For this class of process, no locking is needed at all, since there is
only ever a single thread of execution and the only reentrancy is due to
signal handlers (which are themselves limited to async-signal-safe
operation and should not be doing much work at all).
(2) Single-threaded, calls into OpenSSL after fork()
The application must ensure that it does not fork() with an unexpected
lock held (that is, one that would get unlocked in the parent but
accidentally remain locked in the child and cause deadlock). Since
OpenSSL does not expose any of its internal locks to the application
and the application is single-threaded, the OpenSSL internal locks
will be unlocked for the fork(), and the state will be consistent.
(OpenSSL will need to reseed its PRNG in the child, but that is
an orthogonal issue.) If the application makes use of locks from
libcrypto, proper handling for those locks is the responsibility of
the application, as for any other locking primitive that is available
for application programming.
(3) Multi-threaded, does not call into OpenSSL after fork()
As for (1), the OpenSSL state is only relevant in the parent, so
no particular fork()-related handling is needed. The internal locks
are relevant, but there is no interaction with the child to consider.
(4) Multi-threaded, calls into OpenSSL after fork()
This is the case where the pthread_atfork() hooks to ensure that all
global locks are in a known state across fork() would come into play,
per the above discussion. However, these "calls into OpenSSL after
fork()" are still subject to the restriction to async-signal-safe
functions. Since OpenSSL uses all sorts of locking and libc functions
that are not on the list of safe functions (e.g., malloc()), this
case is not currently usable and is unlikely to ever be usable,
independently of the locking situation. So, there is no need to
go through contortions to attempt to support this case in the one small
area of locking interaction with fork().
In light of the above analysis (thanks @davidben and @achernya), go
back to the simpler implementation that does not need to distinguish
"library-global" locks or to have complicated atfork handling for locks.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5089)
Use atomic operations for the counters
Rename malloc_lock to memdbg_lock
Also fix some style errors in mem_dbg.c
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4359)
[extended tests]
Original text:
Use BUF_strlcpy() instead of strcpy().
Use BUF_strlcat() instead of strcat().
Use BIO_snprintf() instead of sprintf().
In some cases, keep better track of buffer lengths.
This is part of a large change submitted by Markus Friedl <markus@openbsd.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3701)
Travis identified a problem with freeing the ex_data locks which wasn't
quite right in ff2344052. Trying to fix it identified a further problem:
the ex_data locks are cleaned up by OPENSSL_cleanup(), which is called
explicitly by CRYPTO_mem_leaks(), but then later the BIO passed to
CRYPTO_mem_leaks() is freed. An attempt is then made to use the ex_data
lock already freed.
Reviewed-by: Tim Hudson <tjh@openssl.org>
CRYPTO_mem_leaks attempts to adjust the count of bytes leaks to not
include the BIO that is being used to print the results out. However this
does not work properly. In all internal cases we switch off recording
the memory allocation during creation of the BIO so it makes no difference.
In other cases if the BIO allocates any additional memory during
construction then the adjustment will be wrong anyway. It also skips over
the BIO memory during print_leak anyway, so the BIO memory is never
added into the total. In other words this was broken in lots of ways and
has been since it was first added.
The simplest solution is just to make it the documented behaviour that
you must turn off memory logging when creating the BIO, and remove all
the adjustment stuff completely. The adjustment code was only ever in
master and never made it to a release branch so there is no loss of
functionality.
This commit also fixes a compilation failure when using
enable-crypto-mdebug.
Reviewed-by: Rich Salz <rsalz@openssl.org>
- Make use of the functions given through CRYPTO_set_mem_functions().
- CRYPTO_free(), CRYPTO_clear_free() and CRYPTO_secure_free() now receive
__FILE__ and __LINE__.
- The API for CRYPTO_set_mem_functions() and CRYPTO_get_mem_functions()
is slightly changed, the implementation for free() now takes a couple
of extra arguments, taking __FILE__ and __LINE__.
- The CRYPTO_ memory functions will *always* receive __FILE__ and __LINE__
from the corresponding OPENSSL_ macros, regardless of if crypto-mdebug
has been enabled or not. The reason is that if someone swaps out the
malloc(), realloc() and free() implementations, we can't know if they
will use them or not.
Reviewed-by: Rich Salz <rsalz@openssl.org>
To force it on anyone using --strict-warnings was the wrong move, as
this is an option best left to those who know what they're doing.
Use with care!
Reviewed-by: Andy Polyakov <appro@openssl.org>
With the new init framework resources aren't released until the process
exits. This means checking for mem leaks before that point finds a lot of
things! We should explicitly close down the library if we're checking for
mem leaks.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This was done by the following
find . -name '*.[ch]' | /tmp/pl
where /tmp/pl is the following three-line script:
print unless $. == 1 && m@/\* .*\.[ch] \*/@;
close ARGV if eof; # Close file to reset $.
And then some hand-editing of other files.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Make LHASH_OF use static inline functions.
Add new lh_get_down_load and lh_set_down_load functions and their
typesafe inline equivalents.
Make lh_error a function instead of a macro.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Make CRYPTO_mem_leaks() and CRYPTO_mem_leaks_fp() return a status value.
Update documentation. Don't abort() if there are leaks.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
In order for mkdep to find #ifdef'd functions, they must be
wrapped (in the header file) with
#ifndef OPENSSL_NO_...
So do that for various CRYPTO_mem_debug... things.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Only two macros CRYPTO_MDEBUG and CRYPTO_MDEBUG_ABORT to control this.
If CRYPTO_MDEBUG is not set, #ifdef out the whole debug machinery.
(Thanks to Jakob Bohm for the suggestion!)
Make the "change wrapper functions" be the only paradigm.
Wrote documentation!
Format the 'set func' functions so their paramlists are legible.
Format some multi-line comments.
Remove ability to get/set the "memory debug" functions at runtme.
Remove MemCheck_* and CRYPTO_malloc_debug_init macros.
Add CRYPTO_mem_debug(int flag) function.
Add test/memleaktest.
Rename CRYPTO_malloc_init to OPENSSL_malloc_init; remove needless calls.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Rename to OPENSSL_mem_debug_{push,pop}.
Remove simple calls; keep only calls used in recursive functions.
Ensure we always push, to simplify so that we can always pop
Reviewed-by: Richard Levitte <levitte@openssl.org>
Rename BUF_{strdup,strlcat,strlcpy,memdup,strndup,strnlen}
to OPENSSL_{strdup,strlcat,strlcpy,memdup,strndup,strnlen}
Add #define's for the old names.
Add CRYPTO_{memdup,strndup}, called by OPENSSL_{memdup,strndup} macros.
Reviewed-by: Tim Hudson <tjh@openssl.org>
There are header files in crypto/ that are used by a number of crypto/
submodules. Move those to crypto/include/internal and adapt the
affected source code and Makefiles.
The header files that got moved are:
crypto/cryptolib.h
crypto/md32_common.h
Reviewed-by: Rich Salz <rsalz@openssl.org>
For a local variable:
TYPE *p;
Allocations like this are "risky":
p = OPENSSL_malloc(sizeof(TYPE));
if the type of p changes, and the malloc call isn't updated, you
could get memory corruption. Instead do this:
p = OPENSSL_malloc(sizeof(*p));
Also fixed a few memset() calls that I noticed while doing this.
Reviewed-by: Richard Levitte <levitte@openssl.org>
After the finale, the "real" final part. :) Do a recursive grep with
"-B1 -w [a-zA-Z0-9_]*_free" to see if any of the preceeding lines are
an "if NULL" check that can be removed.
Reviewed-by: Tim Hudson <tjh@openssl.org>
OPENSSL_NO_RIPEMD160, OPENSSL_NO_RIPEMD merged into OPENSSL_NO_RMD160
OPENSSL_NO_FP_API merged into OPENSSL_NO_STDIO
Two typo's on #endif comments fixed:
OPENSSL_NO_ECB fixed to OPENSSL_NO_OCB
OPENSSL_NO_HW_SureWare fixed to OPENSSL_NO_HW_SUREWARE
Reviewed-by: Richard Levitte <levitte@openssl.org>
knock-on work than expected - they've been extracted into a patch
series that can be completed elsewhere, or in a different branch,
before merging back to HEAD.
