93 Commits

Author	SHA1	Message	Date
Bernd Edlinger	bea53924ce	Do not use RW mutexes on RISC-V arch ... For unknown reasons using RW mutexes on RISC-V arch seems to be broken, at least with glibc. Fixes #28550 Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> (Merged from https://github.com/openssl/openssl/pull/28634)	2025-09-23 12:45:45 -04:00
Bernd Edlinger	d296f96533	Fix another memory order issue ... this adds another release/acquire link between update_qp and get_hold_current_qp via the reader_idx because the current one which is based on the qp users count is only preventing a race condition, but does not help when the reader acquires the next qp. Fixes #27267 Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/28585)	2025-09-19 07:01:47 +02:00
Neil Horman	036a46d2a4	Fix failure checking on rcu_read_lock ... during memfail testing: https://github.com/openssl/openssl/actions/runs/16794088536/job/47561223902 We get lots of test failures in ossl_rcu_read_lock. This occurs because we have a few cases in the read lock path that attempt mallocs, which, if they fail, trigger an assert or a silent failure, which isn't really appropriate. We should instead fail gracefully, by informing the caller that the lock failed, like we do for CRYPTO_THREAD_read_lock. Fortunately, these are all internal apis, so we can convert ossl_rcu_read_lock to return an int indicating success/failure, and fail gracefully during the test, rather than hitting an assert abort. Fixes openssl/project#1315 Reviewed-by: Paul Yang <paulyang.inf@gmail.com> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> (Merged from https://github.com/openssl/openssl/pull/28195)	2025-08-09 09:22:13 -04:00
Eugene Syromiatnikov	7867bf1523	crypto: use array memory (re)allocation routines ... Co-Authored-by: Alexandr Nedvedicky <sashan@openssl.org> Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from https://github.com/openssl/openssl/pull/28059)	2025-08-08 12:22:10 -04:00
Eugene Syromiatnikov	99d0d23e05	crypto/threads_pthread: rewrite contention data storage to per-tid ... This eliminates locking during writing out of the lock contation report data, which claws back some of the lost performance degradation imposed by the lock contention reporting instrumentation: [Without -DREPORT_RWLOCK_CONTENTION] ~/dev/perftools/source$ ./evp_fetch 100 Average time per fetch call: 4.502162us ~/dev/perftools/source$ ./evp_fetch 200 Average time per fetch call: 8.224920us [Before] ~/dev/perftools/source$ ./evp_fetch 100 Average time per fetch call: 13.079795us ~/dev/perftools/source$ ./evp_fetch 200 Average time per fetch call: 23.420235us [After] ~/dev/perftools/source$ ./evp_fetch 100 Average time per fetch call: 6.557428us ~/dev/perftools/source$ ./evp_fetch 200 Average time per fetch call: 13.415148us The downside is that it produces a file for each TID, which floods the working directory with debug files, but that mich be an acceptable trade-off. Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	b1303b115e	crypto/threads_lock_contention: factor out lock contention recording ... Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	c47c16ee40	crypto/threads_lock_contention: factor out obtaining the stack traces data pointer ... It also drops the premature initalisation of it in ossl_init_rwlock_contention_data(), deferring it to on-demand one in ossl_rwlock_{rd,wr}lock(), which seems to shave some of the incurred overhead: [Before] ~/dev/perftools/source$ ./evp_fetch 100 Average time per fetch call: 16.944004us ~/dev/perftools/source$ ./evp_fetch 200 Average time per fetch call: 26.325767us [After] ~/dev/perftools/source$ ./evp_fetch 100 Average time per fetch call: 13.079795us ~/dev/perftools/source$ ./evp_fetch 200 Average time per fetch call: 23.420235us Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	1178184e96	crypto/threads_lock_contention: condition file suffix on FIPS_MODULE and not fopen() call ... Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	bd0b53a32c	crypto/threads_lock_contention: typo: s/stack_info/stack_traces/ in ossl_init_rwlock_contention_data ... Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	10ce7f45cd	crypto/threads_lock_contention: Remove duplicating code ... Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Eugene Syromiatnikov	e3d98f5bd4	Factor out the lock contention reporting facility implementation ... Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27983)	2025-08-07 11:02:51 -04:00
Norbert Pocs	e740864976	crypto/threads_pthread.c optimize ossl_(un)likely ... Signed-off-by: Norbert Pocs <norbertp@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27961)	2025-07-16 13:07:15 -04:00
Neil Horman	ab021b624f	Add lock contention checking to our pthreads implementation ... Something we're missing in our ability to do performance monitoring and diagnosis in openssl is the ability to check for lock contention. While some tools exist for this (valgrinds drd tool for example), they really only measure the time spent in critical sections, not the instances in which they are contended. For that we need something more specific. This patch introduces the REPORT_RWLOCK_CONTENTION macro. When openssl is built with: ./Configure -rdynamic -fno-omit-frame-pointer -DREPORT_RWLOCK_CONTENTION We can now get output sent to a log file that looks like the following: =============== lock blocked on WRITE for 1001 usec /lib64/libasan.so.8(+0x525e7) [0x7fc5ef4525e7] ../../test/quic_radix_test(CRYPTO_THREAD_write_lock+0x151) [0x79d976] ../../test/quic_radix_test() [0x61e6fb] ../../test/quic_radix_test(test_vprintf_stderr+0x15) [0x61e78d] ../../test/quic_radix_test(test_printf_stderr+0x10f) [0x623ba4] ../../test/quic_radix_test(test_fail_message_prefix+0xf7) [0x62436b] ../../test/quic_radix_test() [0x6243b9] ../../test/quic_radix_test(test_info+0x133) [0x624842] ../../test/quic_radix_test() [0x415aad] ../../test/quic_radix_test() [0x417bad] ../../test/quic_radix_test() [0x41b1a9] ../../test/quic_radix_test() [0x41b2eb] ../../test/quic_radix_test() [0x8071fc] /lib64/libasan.so.8(+0x28ee6) [0x7fc5ef428ee6] /lib64/libc.so.6(+0x711d4) [0x7fc5ef27f1d4] /lib64/libc.so.6(+0xf3cec) [0x7fc5ef301cec] ============== Which tells us when a thread blocked because someone else was already holding the lock, how long it was blocked for, and where the blocking call originated from via its backtrace. I think this should enable us to better determine where our contended locking paths are for a given application, and give us some insight on how to fix them. Currently its linux only (as the backtrace functionality only exists there, and there are few warts (like the need to use a file pointer rather than a bio to record the log, see comments), but I think its enough to give us a useful diagnostic tool to help drive some performance improvements. Fixes openssl/project#1237 Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27884)	2025-07-11 12:38:27 -04:00
Neil Horman	2cb068fb22	update RCU to use the new thread-local key mgmt api ... RCU stores a per-thread local structure per context-thread, making it necessecary to move them to the new api to avoid exhausting our OS level thread-local storage resources when creating lots of contexts Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27794)	2025-06-20 13:01:39 -04:00
Matt Caswell	606de509e3	Assert that we successfully obtained a lock ... We should never fail to obtain a lock, so assert that we are successful Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27408)	2025-04-30 10:55:44 +02:00
Bernd Edlinger	a532f2302d	Do some more cleanup in the RCU code ... Only a minimum of 2 qp's are necessary: one for the readers, and at least one that writers can wait on for retirement. There is no need for one additional qp that is always unused. Also only one ACQUIRE barrier is necessary in get_hold_current_qp, so the ATOMIC_LOAD of the reader_idx can be changed to RELAXED. And finally clarify some comments. Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/27012)	2025-03-18 18:52:29 +01:00
Bernd Edlinger	4a1a7fe5ce	Fix a memory order issue with weakly ordered systems ... this adds a dummy atomic release operation to update_qp, which should make sure that the new value of reader_idx is visible in get_hold_current_qp, directly after incrementing the users count. Fixes: #26875 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26964)	2025-03-17 08:01:26 -04:00
Andrew Dinh	7097d2e00e	Fix RCU TODOs ... - Update allocate_new_qp_group to take unsigned int - Move id_ctr in rcu_lock_st for better stack alignment Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26972)	2025-03-12 12:07:44 -04:00
openssl-machine	0c679f5566	Copyright year updates ... Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Release: yes	2025-03-12 13:35:59 +00:00
Bernd Edlinger	6e7be995fd	RCU: Ensure that qp's are actually retired in order ... The current retirement code for rcu qp's has a race condition, which can cause use-after-free errors, but only if more than 3 QPs are allocated, which is not the default configuration. This fixes an oversight in commit 5949918f9a ("Rework and simplify RCU code") Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26952)	2025-03-05 16:04:16 +01:00
Bernd Edlinger	a6f512a1e6	Revert wrong macos RCU fix ... This reverts #23974 which seems to be no longer needed now, due to other fixes nearby. Most likely the change did just slightly decrease the performance of the reader threads, and did therefore create the wrong impression that it fixed the issue. Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26881)	2025-02-25 10:38:26 +01:00
Bernd Edlinger	3240427a85	Make CRYPTO_atomic_load/store use the same preprocessor guards ... as the other CRYPTO_atomic_X functions. All CRYPTO_atomic functions should use the same logic here, just in case... Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26815)	2025-02-21 14:01:51 +01:00
Bernd Edlinger	eacf14594d	Cleanup atomic fallbacks in threads_pthread.c ... the unused atomic stub functions make clang issue unused function warnings -Wunused-function Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26815)	2025-02-21 14:01:51 +01:00
Neil Horman	7d284560a0	Don't use __ATOMIC_ACQ_REL on older compilers ... Older compilers don't always support __ATOMIC_ACQ_REL, use a lock where they don't Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/26747)	2025-02-16 15:09:03 -05:00
Bernd Edlinger	5949918f9a	Rework and simplify RCU code ... Use __ATOMIC_RELAXED where possible. Dont store additional values in the users field. Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26690)	2025-02-13 20:23:48 +01:00
Bernd Edlinger	65787e2dc2	Revert "rcu: Ensure that updates to the ID field of a qp don't lose refs" ... This reverts commit fbd34c03e3. Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26690)	2025-02-13 20:23:33 +01:00
Neil Horman	fbd34c03e3	rcu: Ensure that updates to the ID field of a qp don't lose refs ... ppc64le occasionally still fails the threadstest on __rcu_torture From several days of debugging, I think I've landed on the problem. Occasionally, under high load I observe the following pattern CPU0 CPU1 update_qp get_hold_current_qp atomic_and_fetch(qp->users, ID_MASK, RELEASE) atomic_add_fetch(qp->users, 1, RELEASE atomic_or_fetch(qp->users, ID_VAL++, RELEASE) When this pattern occurs, the atomic or operation fails to see the published value of CPU1 and when the or-ed value is written back to ram, the incremented value in get_hold_current_qp is overwritten, meaning the hold that the reader placed on the rcu lock is lost, allowing the writer to complete early, freeing memory before a reader is done reading any held memory. Why this is only observed on ppc64le I'm not sure, but it seems like a pretty clear problem. fix it by implementing ATOMIC_COMPARE_EXCHANGE_N, so that, on the write side in update_qp, we can ensure that updates are only done if the read side hasn't changed anything. If it has, retry the operation. With this fix, I'm able to run the threads test overnight (4000 iterations and counting) without failure. Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26478)	2025-01-22 20:16:11 +01:00
Neil Horman	25f8e2c15b	Fix premature reuse of qp's in rcu locks ... An intermittent failure was noted on our new ppc64le CI runner, in which what appeared to be a corrupted or invalid value getting returned from a shared pointer under rcu protection Investigation showed that the problem was with our small number of qp's in a lock, and slightly incorrect accounting of the number of qp's available we were prematurely recycling qp's, which led in turn to premature completion of synchronization states, resulting in readers reading memory that may have already been freed. Fix it by: a) Ensuring that we account for the fact that the first qp in an rcu lock is allocated at the time the lock is created and b) Ensuring that we have a minimum number of 3 qp's: 1 that is free for write side allocation 1 that is in use by the write side currently 1 "next" qp that the read side can update while the prior qp is being retired With this change, the rcu threadstest runs indefinately in my testing Fixes #26356 Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26384)	2025-01-13 17:13:48 -05:00
Tomas Mraz	79c9cbbe1f	Fix breakage on arm64_32 platforms ... We just avoid the special handling needed for Apple M1. Fixes #26135 Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from https://github.com/openssl/openssl/pull/26145)	2024-12-13 15:05:21 +01:00
Neil Horman	4c04a19860	Fix solaris build in CRYPTO_atomic_store api ... Misnamed variable, just correct it to dst Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/26075)	2024-11-29 12:08:10 +01:00
Georgi Valkov	9f4d8c63e8	threads: follow formatting rules ... Adjust long lines and correct padding in preprocessor lines to match the formatting rules Signed-off-by: Georgi Valkov <gvalkov@gmail.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24941)	2024-09-05 17:09:50 +02:00
Neil Horman	7408d58714	disable rwlocks on nonstop klt model ... It appears nonstops new threading model defines some level of rwlock pthread api, but its not working properly. Disable rwlocks for _KLT_MODEL_ for now Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from https://github.com/openssl/openssl/pull/24969)	2024-07-31 08:05:53 -04:00
Neil Horman	3190f5c06b	Fix CRYPTO_atomic_store ... If the implementation of this function falls to using a pthread lock to update a value, it should be a write lock, not a read lock Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from https://github.com/openssl/openssl/pull/24969)	2024-07-31 08:05:53 -04:00
Georgi Valkov	ce6b2f9826	threads_pthread, threads_win: improve code consistency ... Improve code consistency between threads_pthread.c and threads_win.c threads_pthread.c has good comments, let's copy them to threads_win.c In many places uint64_t or LONG int was used, and assignments were performed between variables with different sizes. Unify the code to use uint32_t. In 32 bit architectures it is easier to perform 32 bit atomic operations. The size is large enough to hold the list of operations. Fix result of atomic_or_uint_nv improperly casted to int * instead of int. Note: In general size_t should be preferred for size and index, due to its descriptive name, however it is more convenient to use uint32_t for consistency between platforms and atomic calls. READER_COUNT and ID_VAL return results that fit 32 bit. Cast them to uint32_t to save a few CPU cycles, since they are used in 32 bit operations anyway. TODO: In struct rcu_lock_st, qp_group can be moved before id_ctr for better alignment, which would save 8 bytes. allocate_new_qp_group has a parameter count of type int. Signed values should be avoided as size or index. It is better to use unsigned, e.g uint32_t, even though internally this is assigned to a uint32_t variable. READER_SIZE is 16 in threads_pthread.c, and 32 in threads_win.c Using a common size for consistency should be prefered. Signed-off-by: Georgi Valkov <gvalkov@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24803)	2024-07-17 16:37:07 +02:00
Georgi Valkov	16beec98d2	threads_win: fix build error with VS2010 x86 ... InterlockedAnd64 and InterlockedAdd64 are not available on VS2010 x86. We already have implemented replacements for other functions, such as InterlockedOr64. Apply the same approach to fix the errors. A CRYPTO_RWLOCK rw_lock is added to rcu_lock_st. Replace InterlockedOr64 and InterlockedOr with CRYPTO_atomic_load and CRYPTO_atomic_load_int, using the existing design pattern. Add documentation and tests for the new atomic functions CRYPTO_atomic_add64, CRYPTO_atomic_and Fixes: libcrypto.lib(libcrypto-lib-threads_win.obj) : error LNK2019: unresolved external symbol _InterlockedAdd64 referenced in function _get_hold_current_qp libcrypto.lib(libcrypto-lib-threads_win.obj) : error LNK2019: unresolved external symbol _InterlockedOr referenced in function _get_hold_current_qp libcrypto.lib(libcrypto-lib-threads_win.obj) : error LNK2019: unresolved external symbol _InterlockedAnd64 referenced in function _update_qp libcrypto.lib(libcrypto-lib-threads_win.obj) : error LNK2019: unresolved external symbol _InterlockedOr64 referenced in function _ossl_synchronize_rcu Signed-off-by: Georgi Valkov <gvalkov@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24405)	2024-07-01 10:02:02 +02:00
Neil Horman	f7252d736d	Some minor nit corrections in the thread code for rcu ... Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24630) (cherry picked from commit d38d264228)	2024-06-20 16:56:39 +02:00
sanumesh	36ba419286	threads_pthread.c: change inline to ossl_inline ... Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24502)	2024-05-28 17:19:18 +02:00
Neil Horman	7e45ac6891	Add CRYPTO_atomic_store api ... Generally we can get away with just using CRYPTO_atomic_load to do stores by reversing the source and target variables, but doing so creates a problem for the thread sanitizer as CRYPTO_atomic_load hard codes an __ATOMIC_ACQUIRE constraint, which confuses tsan into thinking that loads and stores aren't properly ordered, leading to RAW/WAR hazzards getting reported. Instead create a CRYPTO_atomic_store api that is identical to the load variant, save for the fact that the value is a unit64_t rather than a pointer that gets stored using an __ATOMIC_RELEASE constraint, satisfying tsan. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23671)	2024-04-24 12:03:03 +10:00
Neil Horman	f39a862818	Fix list appending in win ossl_rcu_call ... The ossl_rcu_call function for windows creates a linked list loop. fix it to work like the pthread version properly Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23671)	2024-04-24 12:03:03 +10:00
Neil Horman	3bcac46035	Make thread sanitizer cope with rcu locks ... This is unfortunate, but seems necessecary tsan in gcc/clang tracks data races by recording memory references made while various locks are held. If it finds that a given address is read/written while under lock (or under no locks without the use of atomics), it issues a warning this creates a specific problem for rcu, because on the write side of a critical section, we write data under the protection of a lock, but by definition the read side has no lock, and so rcu warns us about it, which is really a false positive, because we know that, even if a pointer changes its value, the data it points to will be valid. The best way to fix it, short of implementing tsan hooks for rcu locks in any thread sanitizer in the field, is to 'fake it'. If thread sanitization is activated, then in ossl_rcu_write_[lock|unlock] we add annotations to make the sanitizer think that, after the write lock is taken, that we immediately unlock it, and lock it right before we unlock it again. In this way tsan thinks there are no locks held while referencing protected data on the read or write side. we still need to use atomics to ensure that tsan recognizes that we are doing atomic accesses safely, but thats ok, and we still get warnings if we don't do that properly Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23671)	2024-04-24 12:03:03 +10:00
Logan Upchurch	5454ef7cb3	crypto/threads_pthread.c: Fix typos found by codespell ... CLA: trivial Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24206)	2024-04-23 11:26:29 +02:00
Neil Horman	24d16d3a19	Make rcu_thread_key context-aware ... Currently, rcu has a global bit of data, the CRYPTO_THREAD_LOCAL object to store per thread data. This works in some cases, but fails in FIPS, becuase it contains its own copy of the global key. So 1) Make the rcu_thr_key a per-context variable, and force ossl_rcu_lock_new to be context aware 2) Store a pointer to the context in the lock object 3) Use the context to get the global thread key on read/write lock 4) Use ossl_thread_start_init to properly register a cleanup on thread exit 5) Fix up missed calls to OSSL_thread_stop() in our tests Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24162)	2024-04-19 09:22:53 -04:00
Richard Levitte	a02077d4d7	crypto/threads_pthread.c: refactor all atomics fallbacks for type safety ... The atomics fallbacks were using 'void *' as a generic transport for all possible scalar and pointer types, with the hypothesis that a pointer is as large as the largest possible scalar type that we would use. Then enters the use of uint64_t, which is larger than a pointer on any 32-bit system (or any system that has 32-bit pointer configurations). We could of course choose a larger type as a generic transport. However, that only pushes the problem forward in time... and it's still a hack. It's therefore safer to reimplement the fallbacks per type that atomics are used for, and deal with missing per type fallbacks when the need arrises in the future. For test build purposes, the macro USE_ATOMIC_FALLBACKS is introduced. If OpenSSL is configured with '-DUSE_ATOMIC_FALLBACKS', the fallbacks will be used, unconditionally. Fixes #24096 Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24123)	2024-04-16 09:18:01 +02:00
Richard Levitte	81f393498b	crypto/threads_pthread.c: Cleanup misaligned preprocessor directives ... Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24123)	2024-04-16 09:18:01 +02:00
Neil Horman	f5b5a35c84	Ensure proper memory barriers around ossl_rcu_deref/ossl_rcu_assign_ptr ... Since the addition of macos14 M1 runners in our CI jobs we've been seeing periodic random failures in the test_threads CI job. Specifically we've seen instances in which the shared pointer in the test (which points to a monotonically incrementing uint64_t went backwards. From taking a look at the disassembled code in the failing case, we see that __atomic_load_n when emitted in clang 15 looks like this 0000000100120488 <_ossl_rcu_uptr_deref>: 100120488: f8bfc000 ldapr x0, [x0] 10012048c: d65f03c0 ret Notably, when compiling with gcc on the same system we get this output instead: 0000000100120488 <_ossl_rcu_uptr_deref>: 100120488: f8bfc000 ldar x0, [x0] 10012048c: d65f03c0 ret Checking the arm docs for the difference between ldar and ldapr: https://developer.arm.com/documentation/ddi0602/2023-09/Base-Instructions/LDAPR--Load-Acquire-RCpc-Register- https://developer.arm.com/documentation/dui0802/b/A64-Data-Transfer-Instructions/LDAR It seems that the ldar instruction provides a global cpu fence, not completing until all writes in a given cpus writeback queue have completed Conversely, the ldapr instruction attmpts to achieve performance improvements by honoring the Local Ordering register available in the system coprocessor, only flushing writes in the same address region as other cpus on the system. I believe that on M1 virtualized cpus the ldapr is not properly ordering writes, leading to an out of order read, despite the needed fencing. I've opened an issue with apple on this here: https://developer.apple.com/forums/thread/749530 I believe that it is not safe to issue an ldapr instruction unless the programmer knows that the Local order registers are properly configured for use on the system. So to fix it I'm proposing with this patch that we, in the event that: 1) __APPLE__ is defined AND 2) __clang__ is defined AND 3) __aarch64__ is defined during the build, that we override the ATOMIC_LOAD_N macro in the rcu code such that it uses a custom function with inline assembly to emit the ldar instruction rather than the ldapr instruction. The above conditions should get us to where this is only used on more recent MAC cpus, and only in the case where the affected clang compiler emits the offending instruction. I've run this patch 10 times in our CI and failed to reproduce the issue, whereas previously I could trigger it within 5 runs routinely. Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23974)	2024-04-10 09:18:07 +02:00
Richard Levitte	b646179229	Copyright year updates ... Reviewed-by: Neil Horman <nhorman@openssl.org> Release: yes (cherry picked from commit 0ce7d1f355) Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/24034)	2024-04-09 13:43:26 +02:00
Dimitri Papadopoulos	39fe3e5de1	Fix the grammar as suggsted in the review ... Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23545)	2024-02-14 09:36:11 +01:00
Dimitri Papadopoulos	f7241edda4	Fix new typos found by codespell ... Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23545)	2024-02-14 09:36:11 +01:00
Neil Horman	d0e1a0ae70	RCU lock implementation ... Introduce an RCU lock implementation as an alternative locking mechanism to openssl. The api is documented in the ossl_rcu.pod file Read side implementaiton is comparable to that of RWLOCKS: ossl_rcu_read_lock(lock); < critical section in which data can be accessed via ossl_derefrence > ossl_rcu_read_unlock(lock); Write side implementation is: ossl_rcu_write_lock(lock); < critical section in which data can be updated via ossl_assign_pointer and stale data can optionally be scheduled for removal via ossl_rcu_call > ossl_rcu_write_unlock(lock); ... ossl_synchronize_rcu(lock); ossl_rcu_call fixup Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/22729)	2024-02-01 08:33:25 -05:00
Matt Caswell	da1c088f59	Copyright year updates ... Reviewed-by: Richard Levitte <levitte@openssl.org> Release: yes	2023-09-07 09:59:15 +01:00