aboutsummaryrefslogtreecommitdiffstats
path: root/include
diff options
context:
space:
mode:
authorNeil Horman <[email protected]>2026-09-03 22:50:20 +0200
committerNeil Horman <[email protected]>2026-09-11 22:50:54 +0200
commitf99bbcd5afff69840f2bfed92458cd5ac092cf91 (patch)
tree0064dfa7620da14636ca721bffe363b117a53374 /include
parentRevert "Free connections before listeners in the radix test cleanup" (diff)
downloadopenssl-f99bbcd5afff69840f2bfed92458cd5ac092cf91.tar.gz
openssl-f99bbcd5afff69840f2bfed92458cd5ac092cf91.zip
Dont have dgram_pairs track bios, track bio_dgram_pair_st instead
@ifanzki reported an asan issue with the quic_radix_test in: https://github.com/openssl/openssl/issues/32420 And we've tried fixing it a few different ways, with varying amounts of success This commit represents an alternative approach which I think is more general to the bio_dgram_pair bio type. The problem: We have a race condition as reported above in which freeing one side of a BIO dgram pair results in a use after free The root cause: BIO_new_bio_dgram_pair creates 2 dgram BIOS, each with a reference count of one (which is sensible, as each bio has a single owner). However, after creating the bios, it calls BIO_make_bio_pair(), which links the two together. Under the covers in the bio_dgram_pair implementation, each bio stores the pointer of its peer bio _without_ taking an additional reference on the bio. This means that, once the first bio is freed, the second bio may be doing subsequent io, which requires dereferencing of the peer bio , which is already release, leading to the use after free case observed. Nominally this isn't an issue, as most code that uses BIO_dgram_pairs are single threaded, and freeing of the component bios only occurs after any i/o is already terminated, avoiding the observed UAF. However, the QUIC radix test exercises the thread assisted mode of the quic stack, and in so doing passes each half of a bio dgram pair to a separate thread. The fix: Nominally we could resolve this by increasing the reference count on each of the peer bios, and dropping that ref count when each is freed, so that only the last to free actually deallocates both instances, but this creates a chicken and egg issue. Since the the stored reference to each bio lives in the dgram pair implementation, upping the refcount of the bios prevents either bio from ever reaching the implementation free code (dgram_pair_free) So instead I'm proposing the following in this commit: 1) Instead of having the internal bio data track the peer bio, instead have the internal data track a shared bio_dgram_peer_st structure which is reference counted 2) When creating a pair, the pair only uses the data (ring buffers, locks, etc), from the shared data, as its guaranteed to live until the last owner of the pair either frees itself BIO_free, or destroys the pair vi BIO_dgram_destroy_pair 3) If either side of the pair disconnects via the calls in (3), the shared data state is updated to reflect that, and the remaining peer triggers on that to get BROKEN_PIPE return codes Also added tests from @bukka, as they provide more coverage than the test I had in mind. Fixes #32420 Reviewed-by: Jakub Zelenka <[email protected]> Reviewed-by: Andrew Dinh <[email protected]> MergeDate: Fri Sep 11 20:50:58 2026 (Merged from https://github.com/openssl/openssl/pull/32498)
Diffstat (limited to 'include')
-rw-r--r--include/internal/threads_common.h2
1 files changed, 2 insertions, 0 deletions
diff --git a/include/internal/threads_common.h b/include/internal/threads_common.h
index e32257d6301..c1da68a5dbd 100644
--- a/include/internal/threads_common.h
+++ b/include/internal/threads_common.h
@@ -14,9 +14,11 @@
#if defined(__clang__) && defined(__has_feature)
#if __has_feature(thread_sanitizer)
+#if !defined(__SANITIZE_THREAD__)
#define __SANITIZE_THREAD__
#endif
#endif
+#endif
#if defined(__SANITIZE_THREAD__)
#include <sanitizer/tsan_interface.h>