[openssl/openssl] d09220: Fix potential divide by zero error

Randall S. Becker noreply at github.com
Wed Apr 24 12:47:40 UTC 2024


  Branch: refs/heads/master
  Home:   https://github.com/openssl/openssl
  Commit: d092208bd694c9f9b276965bcf2d33f164535b2f
      https://github.com/openssl/openssl/commit/d092208bd694c9f9b276965bcf2d33f164535b2f
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M test/threadstest.c

  Log Message:
  -----------
  Fix potential divide by zero error

Coverity caught the following issues:
1591477
1591475
1591473
1591470

all of which are simmilar, in that they catch potential divide by zero
in double values.  It can't actually happen since the the threads which
increment these counters don't exit until they reach non-zero values,
but its easy to add the checks, so lets do that to ensure that we don't
change something in the future that causes it.

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23462)


  Commit: a928f26813e41018d364a5178c53ebb6d49d3e59
      https://github.com/openssl/openssl/commit/a928f26813e41018d364a5178c53ebb6d49d3e59
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M crypto/conf/conf_mod.c

  Log Message:
  -----------
  Coverity found the following issues:

1591471
1591474
1591476

which pertain to memory leaks in the conf_mod code

If an error is encountered after the module STACK_OF is duplicated or
created in the new_modules variable, we need to remember to free it in
the error path

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23462)


  Commit: 3bcac46035d16e777c6651c18078bbcab27ad17a
      https://github.com/openssl/openssl/commit/3bcac46035d16e777c6651c18078bbcab27ad17a
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M crypto/threads_pthread.c

  Log Message:
  -----------
  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 at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: f39a86281883bd7ff0b3791ed203756d055c001b
      https://github.com/openssl/openssl/commit/f39a86281883bd7ff0b3791ed203756d055c001b
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M crypto/threads_pthread.c
    M crypto/threads_win.c

  Log Message:
  -----------
  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 at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: 7e45ac6891ade57cb0141402745d144c4ce342cb
      https://github.com/openssl/openssl/commit/7e45ac6891ade57cb0141402745d144c4ce342cb
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M crypto/threads_none.c
    M crypto/threads_pthread.c
    M crypto/threads_win.c
    M doc/man3/CRYPTO_THREAD_run_once.pod
    M include/openssl/crypto.h.in
    M util/libcrypto.num

  Log Message:
  -----------
  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 at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: cc4ea5e00028e8e0fe3acbf5027497c077f84446
      https://github.com/openssl/openssl/commit/cc4ea5e00028e8e0fe3acbf5027497c077f84446
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M crypto/build.info
    A crypto/hashtable/build.info
    A crypto/hashtable/hashtable.c
    M crypto/mem.c
    A doc/internal/man3/ossl_ht_new.pod
    M doc/man3/CRYPTO_THREAD_run_once.pod
    M doc/man3/OPENSSL_malloc.pod
    A include/internal/hashtable.h
    M include/openssl/crypto.h.in
    M test/build.info
    M test/lhash_test.c
    M util/libcrypto.num
    M util/other.syms
    M util/platform_symbols/unix-symbols.txt

  Log Message:
  -----------
  Introduce new internal hashtable implementation

Create a new hashtable that is more efficient than the existing LHASH_OF
implementation.  the new ossl_ht api offers several new features that
improve performance opportunistically

* A more generalized hash function.  Currently using fnv1a, provides a
  more general hash function, but can still be overridden where needed

* Improved locking and reference counting.  This hash table is
  internally locked with an RCU lock, and optionally reference counts
  elements, allowing for users to not have to create and manage their
  own read/write locks

* Lockless operation.  The hash table can be configured to operate
  locklessly on the read side, improving performance, at the sacrifice
  of the ability to grow the hash table or delete elements from it

* A filter function allowing for the retrieval of several elements at a
  time matching a given criteria without having to hold a lock
  permanently

* a doall_until iterator variant, that allows callers which need to
  iterate over the entire hash table until a given condition is met (as
  defined by the return value of the iterator callback).  This allows
  for callers attempting to do expensive cache searches for a small
  number of elements to terminate the iteration early, saving cpu cycles

* Dynamic type safety.  The hash table provides operations to set and
  get data of a specific type without having to define a type at the
  instatiation point

* Multiple data type storage.  The hash table can store multiple data
  types allowing for more flexible usage

* Ubsan safety.  Because the API deals with concrete single types
  (HT_KEY and HT_VALUE), leaving specific type casting to the call
  recipient with dynamic type validation, this implementation is safe
  from the ubsan undefined behavior warnings that require additional
  thunking on callbacks.

Testing of this new hashtable with an equivalent hash function, I can
observe approximately a 6% performance improvement in the lhash_test

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: f597acb71b67bfa8f2e342301ebce2059408ac27
      https://github.com/openssl/openssl/commit/f597acb71b67bfa8f2e342301ebce2059408ac27
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M fuzz/build.info
    A fuzz/hashtable.c
    A test/recipes/99-test_fuzz_hashtable.t

  Log Message:
  -----------
  Adding hashtable fuzzer

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: 2a54ec0bdd76e93d2c1d92fc0b8e261ac0cea12d
      https://github.com/openssl/openssl/commit/2a54ec0bdd76e93d2c1d92fc0b8e261ac0cea12d
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M doc/internal/man3/ossl_ht_new.pod
    M test/lhash_test.c

  Log Message:
  -----------
  adding a multithreaded hashtable test

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: ca43171b3c38cd8bcd6de8ec11a3b34751cd5a8b
      https://github.com/openssl/openssl/commit/ca43171b3c38cd8bcd6de8ec11a3b34751cd5a8b
  Author: Neil Horman <nhorman at openssl.org>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M fuzz/corpora

  Log Message:
  -----------
  updating fuzz-corpora submodule

Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)


  Commit: 0339382abad578ccb3989799ea2fb99dfb2d099b
      https://github.com/openssl/openssl/commit/0339382abad578ccb3989799ea2fb99dfb2d099b
  Author: Randall S. Becker <randall.becker at nexbridge.ca>
  Date:   2024-04-24 (Wed, 24 Apr 2024)

  Changed paths:
    M NOTES-NONSTOP.md
    M apps/lib/apps.c
    M apps/lib/http_server.c
    M apps/lib/s_socket.c
    M apps/ocsp.c
    M apps/speed.c
    M crypto/bio/bio_sock.c
    M include/internal/sockets.h
    M test/drbgtest.c

  Log Message:
  -----------
  Remove all references to FLOSS for NonStop Builds.

FLOSS is no longer a dependency for NonStop as of the deprecation of the SPT
thread model builds.

Fixes: #24214

Signed-off-by: Randall S. Becker <randall.becker at nexbridge.ca>

Reviewed-by: Tom Cosgrove <tom.cosgrove at arm.com>
Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24217)


Compare: https://github.com/openssl/openssl/compare/264ff64b9443...0339382abad5

To unsubscribe from these emails, change your notification settings at https://github.com/openssl/openssl/settings/notifications


More information about the openssl-commits mailing list