[pull] master from openssl:master#197
Merged
pull[bot] merged 4 commits intoabitmore:masterfrom Oct 23, 2025
Merged
Conversation
PR #24782 introduced a copying of the algs stack in ossl_method_store_do all, so that the subsequent iteration of elements through alg_do_one could be done without a lock, and without triggering a tsan error as reported in: #24672 However, the problem wasn't completely fixed. Issue: #27726 Noted that, sometimes we still get a crash when iterating over each algs impls stack. This occurs because, even though we've cloned the algs to a private data area, the impls stack for each alg still points to shared data, which we are accessing without the benefit of a lock. Because of that, if some other thread calls a function that mutates the impl stack (say ossl_method_store_remove()), we may encounter a NULL or garbage value in one of the impl stack values, leading to an unexpected NULL pointer or simmilar, which in turn leads to a crash. Unfortunately we can't use a lock to create exclusive access here, as there are several paths that lead to a recursive mutation of the stack, which would deadlock. So the only way that I see to prevent this (which is admittedly ugly), is to not only clone the alg stack, but to duplicate each algs impl stack with the read lock held, prior to doing the iteration. Further, we've been unable to test this, as the problem is rare, and we don't have a solid reproducer for the issue, but visual inspection suggests this should fix that. Hopefully: Fixes #27726 Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #28783)
If the call to ASN1_item_ex_d2i() from x509_pubkey_ex_d2i_ex() fails *pval is freed by asn1_item_ex_d2i_intern()->ASN1_item_ex_free()->ossl_asn1_item_embed_free() inside the ASN1_item_ex_d2i() function without freeing the string buffer X509_PUBKEY::propq that was previously allocated in x509_pubkey_ex_new_ex() and we lose the pointer to this buffer. The function we are fixing here is one of the functions used to define X509_PUBKEY - so any operations that work directly on X509_PUBKEY_INTERNAL should be prevented from freeing the structure because they don't know how to handle it. Signed-off-by: Andrey Tsygunka <aitsygunka@yandex.ru> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #27333)
…EAD_clean_local() Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #28781)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )