openssl for i586 #31793
Replies: 15 comments 8 replies
-
|
What do you get from |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
Are you able to build a debug version of OpenSSL with a custom config option? If so can you rebuild OpenSSL with the "enable-trace" option switched on? Once built rerun the same command you tried above just to confirm that it is still failing in the same way. Then try: Please post the output you get here. |
Beta Was this translation helpful? Give feedback.
-
|
TRACE[00:F7:74:B7]:PROVIDER: (provider default) initializing |
Beta Was this translation helpful? Give feedback.
-
|
That's very interesting. This seems to be pointing at a possible OpenSSL bug somewhere. Although why it is only impacting you is a bit of a mystery. I have prepared a debug patch to give us some more information about where exactly the problem lies. Are you able apply the patch, rebuild and rerun the command (including the OPENSSL_TRACE part) and post the output: From ae728be13f6e6349fa3c393cc31e6fb27e587640 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.foundation>
Date: Thu, 2 Jul 2026 09:26:28 +0100
Subject: [PATCH] Debug file store fetching
---
crypto/property/property.c | 46 ++++++++++++++++++++++++++++++++------
crypto/store/store_meth.c | 21 +++++++++++++----
2 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/crypto/property/property.c b/crypto/property/property.c
index b702d03f61..7a3e4ca54f 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -324,8 +324,21 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
int ret = 0;
int i;
- if (nid <= 0 || method == NULL || store == NULL)
+ fprintf(stderr, "DEBUG ossl_method_store_add: nid=%d properties=%s store=%p method=%p\n",
+ nid, properties ? properties : "(null)", (void *)store, method);
+
+ if (nid <= 0) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: nid <= 0, returning 0\n");
+ return 0;
+ }
+ if (method == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: method is NULL, returning 0\n");
+ return 0;
+ }
+ if (store == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: store is NULL, returning 0\n");
return 0;
+ }
if (properties == NULL)
properties = "";
@@ -335,12 +348,15 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
/* Create new entry */
impl = OPENSSL_malloc(sizeof(*impl));
- if (impl == NULL)
+ if (impl == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: OPENSSL_malloc(impl) failed\n");
return 0;
+ }
impl->method.method = method;
impl->method.up_ref = method_up_ref;
impl->method.free = method_destruct;
if (!ossl_method_up_ref(&impl->method)) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: ossl_method_up_ref failed\n");
OPENSSL_free(impl);
return 0;
}
@@ -348,6 +364,7 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
/* Insert into the hash table if required */
if (!ossl_property_write_lock(store)) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: ossl_property_write_lock failed\n");
impl_free(impl);
return 0;
}
@@ -372,9 +389,14 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
*/
if ((impl->properties = ossl_prop_defn_get(store->ctx, properties)) == NULL) {
impl->properties = ossl_parse_property(store->ctx, properties);
- if (impl->properties == NULL)
+ if (impl->properties == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: ossl_parse_property failed for '%s'\n",
+ properties);
goto err;
+ }
if (!ossl_prop_defn_set(store->ctx, properties, &impl->properties)) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: ossl_prop_defn_set failed for '%s'\n",
+ properties);
ossl_property_free(impl->properties);
impl->properties = NULL;
goto err;
@@ -387,13 +409,23 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
*/
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL) {
- if ((alg = OPENSSL_zalloc(sizeof(*alg))) == NULL
- || (alg->impls = sk_IMPLEMENTATION_new_null()) == NULL
- || (alg->cache = lh_QUERY_new(&query_hash, &query_cmp)) == NULL)
+ if ((alg = OPENSSL_zalloc(sizeof(*alg))) == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: OPENSSL_zalloc(alg) failed\n");
+ goto err;
+ }
+ if ((alg->impls = sk_IMPLEMENTATION_new_null()) == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: sk_IMPLEMENTATION_new_null failed\n");
goto err;
+ }
+ if ((alg->cache = lh_QUERY_new(&query_hash, &query_cmp)) == NULL) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: lh_QUERY_new failed\n");
+ goto err;
+ }
alg->nid = nid;
- if (!ossl_method_store_insert(store, alg))
+ if (!ossl_method_store_insert(store, alg)) {
+ fprintf(stderr, "DEBUG ossl_method_store_add: ossl_method_store_insert failed\n");
goto err;
+ }
OSSL_TRACE2(QUERY, "Inserted an alg with nid %d into the store %p\n", nid, (void *)store);
}
diff --git a/crypto/store/store_meth.c b/crypto/store/store_meth.c
index 09252df009..d4c93b0ce5 100644
--- a/crypto/store/store_meth.c
+++ b/crypto/store/store_meth.c
@@ -7,6 +7,7 @@
* https://www.openssl.org/source/license.html
*/
+#include <stdio.h>
#include <openssl/crypto.h>
#include "crypto/store.h"
#include "internal/core.h"
@@ -163,16 +164,28 @@ static int put_loader_in_store(void *store, void *method,
struct loader_data_st *methdata = data;
OSSL_NAMEMAP *namemap;
int id;
+ int ret;
- if ((namemap = ossl_namemap_stored(methdata->libctx)) == NULL
- || (id = ossl_namemap_name2num(namemap, scheme)) == 0)
+ if ((namemap = ossl_namemap_stored(methdata->libctx)) == NULL) {
+ fprintf(stderr, "DEBUG put_loader_in_store: namemap is NULL\n");
return 0;
+ }
+ if ((id = ossl_namemap_name2num(namemap, scheme)) == 0) {
+ fprintf(stderr, "DEBUG put_loader_in_store: namemap lookup for '%s' returned 0\n",
+ scheme);
+ return 0;
+ }
+ fprintf(stderr, "DEBUG put_loader_in_store: scheme '%s' -> id %d\n", scheme, id);
- if (store == NULL && (store = get_loader_store(methdata->libctx)) == NULL)
+ if (store == NULL && (store = get_loader_store(methdata->libctx)) == NULL) {
+ fprintf(stderr, "DEBUG put_loader_in_store: loader store is NULL\n");
return 0;
+ }
- return ossl_method_store_add(store, prov, id, propdef, method,
+ ret = ossl_method_store_add(store, prov, id, propdef, method,
up_ref_loader, free_loader);
+ fprintf(stderr, "DEBUG put_loader_in_store: ossl_method_store_add returned %d\n", ret);
+ return ret;
}
static void *loader_from_algorithm(int scheme_id, const OSSL_ALGORITHM *algodef,
--
2.43.0 |
Beta Was this translation helpful? Give feedback.
-
|
I applied the patch and re-ran the command. |
Beta Was this translation helpful? Give feedback.
-
|
um: I think that uri is incorrect. uri_file_stat() expects that the uri either be a direct file name (i.e. /tmp/asdf), or a fully qualified file uri (i.e. file:///tmp/asdf) |
Beta Was this translation helpful? Give feedback.
-
That's not the issue here. (I mean it is an issue, but it is not the issue). We should not be seeing the No store loader found error. We seem to have the case where |
Beta Was this translation helpful? Give feedback.
-
|
@uglyoldbob thanks for applying the patch and running it. It gives more details about where the failure occurs, but we need to narrow in a bit further. Can you apply the following (on top of the previous patch) and repeat? From ae266951e82b9d5056b7af474a93e380a882953b Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.foundation>
Date: Fri, 3 Jul 2026 16:16:20 +0100
Subject: [PATCH] Add deeper namemap/hashtable diagnostics for i586 file-store
bug
Trace the namemap insert/lookup for the 'file' scheme: struct sizes,
key buffer, computed hash, and an immediate reverify after insert.
Gated on 'file' to keep the output small.
Assisted-by: Claude Code:claude-opus-4-8
---
crypto/core_namemap.c | 35 +++++++++++++++++++++++++++++++++++
crypto/property/property.c | 3 ---
crypto/store/store_meth.c | 10 +++++++++-
3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/crypto/core_namemap.c b/crypto/core_namemap.c
index 8e646c3bcc..f6677c97b9 100644
--- a/crypto/core_namemap.c
+++ b/crypto/core_namemap.c
@@ -7,9 +7,11 @@
* https://www.openssl.org/source/license.html
*/
+#include <stdio.h>
#include "internal/namemap.h"
#include "internal/tsan_assist.h"
#include "internal/hashtable.h"
+#include "internal/hashfunc.h"
#include "internal/sizes.h"
#include "crypto/context.h"
@@ -272,8 +274,25 @@ static int namemap_add_name(OSSL_NAMEMAP *namemap, int number,
HT_INIT_KEY(&key);
HT_SET_KEY_STRING_CASE(&key, name, name);
+
+ /* Only trace the scheme we're chasing, to keep the output manageable */
+ if (strcmp(name, "file") == 0)
+ fprintf(stderr, "DEBUG namemap_add_name: namemap=%p ht=%p inserting '%s' "
+ "sizeof(HT_KEY)=%zu sizeof(NAMENUM_KEY)=%zu "
+ "keysize=%zu keybuf=%p hash=%llu\n",
+ (void *)namemap, (void *)namemap->namenum_ht, name,
+ sizeof(HT_KEY), sizeof(NAMENUM_KEY),
+ (size_t)key.key_header.keysize,
+ (void *)key.key_header.keybuf,
+ (unsigned long long)ossl_fnv1a_hash(key.key_header.keybuf,
+ key.key_header.keysize));
+
val.value = (void *)(intptr_t)number;
ret = ossl_ht_insert(namemap->namenum_ht, TO_HT_KEY(&key), &val, NULL);
+
+ if (strcmp(name, "file") == 0)
+ fprintf(stderr, "DEBUG namemap_add_name: insert rc=%d\n", ret);
+
if (!ossl_assert(ret != 0)) /* cannot happen as we are under write lock */
return 0;
if (ret < 1) {
@@ -281,6 +300,22 @@ static int namemap_add_name(OSSL_NAMEMAP *namemap, int number,
ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_MANY_NAMES);
return 0;
}
+
+ /* Immediately verify the lookup works */
+ if (strcmp(name, "file") == 0) {
+ NAMENUM_KEY lkey;
+ int verify;
+
+ HT_INIT_KEY(&lkey);
+ HT_SET_KEY_STRING_CASE(&lkey, name, name);
+ fprintf(stderr, "DEBUG namemap_add_name: lookup key keysize=%zu keybuf=%p hash=%llu\n",
+ (size_t)lkey.key_header.keysize, (void *)lkey.key_header.keybuf,
+ (unsigned long long)ossl_fnv1a_hash(lkey.key_header.keybuf,
+ lkey.key_header.keysize));
+ verify = ossl_namemap_name2num(namemap, name);
+ fprintf(stderr, "DEBUG namemap_add_name: immediate reverify '%s' -> id=%d\n",
+ name, verify);
+ }
return number;
}
diff --git a/crypto/property/property.c b/crypto/property/property.c
index 7a3e4ca54f..37a0240a79 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -324,9 +324,6 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
int ret = 0;
int i;
- fprintf(stderr, "DEBUG ossl_method_store_add: nid=%d properties=%s store=%p method=%p\n",
- nid, properties ? properties : "(null)", (void *)store, method);
-
if (nid <= 0) {
fprintf(stderr, "DEBUG ossl_method_store_add: nid <= 0, returning 0\n");
return 0;
diff --git a/crypto/store/store_meth.c b/crypto/store/store_meth.c
index d4c93b0ce5..74bde3799c 100644
--- a/crypto/store/store_meth.c
+++ b/crypto/store/store_meth.c
@@ -170,6 +170,8 @@ static int put_loader_in_store(void *store, void *method,
fprintf(stderr, "DEBUG put_loader_in_store: namemap is NULL\n");
return 0;
}
+ fprintf(stderr, "DEBUG put_loader_in_store: scheme='%s' namemap=%p\n",
+ scheme, (void *)namemap);
if ((id = ossl_namemap_name2num(namemap, scheme)) == 0) {
fprintf(stderr, "DEBUG put_loader_in_store: namemap lookup for '%s' returned 0\n",
scheme);
@@ -275,9 +277,15 @@ static void *construct_loader(const OSSL_ALGORITHM *algodef,
OSSL_LIB_CTX *libctx = ossl_provider_libctx(prov);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
const char *scheme = algodef->algorithm_names;
- int id = ossl_namemap_add_name(namemap, 0, scheme);
+ int id;
void *method = NULL;
+ fprintf(stderr, "DEBUG construct_loader: scheme='%s' namemap=%p libctx=%p\n",
+ scheme, (void *)namemap, (void *)libctx);
+ id = ossl_namemap_add_name(namemap, 0, scheme);
+ fprintf(stderr, "DEBUG construct_loader: ossl_namemap_add_name('%s') -> id=%d\n",
+ scheme, id);
+
if (id != 0)
method = loader_from_algorithm(id, algodef, prov);
--
2.43.0 |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
Just to confirm @uglyoldbob your tests above, are you testing on 3.5.5 or 3.5.6? I ask because we fixed an alignment issue in the hashtable code (which impacted the namemap) in commit 75a3b8f, which impacted 32 bit systems. I know you said this was happening on both 3.5.5 and 3.5.6, but its probably worth restricting your testing to 3.5.6 specifically, just to ensure thats not causing a problem. Additionally, the hashtable is optimized by ensuring that all the hopscotch tables are sized and aligned to match the cache line size of the cpu its executing on, which is, in almost all modern cpus, 64 bytes. But IIRC i586 has a 32 byte cache line. It might be worth applying this patch: Thats not a real fix of course, we'll have to make that conditional on the arch being built for, but it would be a good data point to have, in the event that a neighborhood in the hashtable is spanning a cacheline, and thats somehow impacting the atomic operations that the hashtable uses. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks @uglyoldbob. This seems to point to some weirdness in the hashtable implementation. @nhorman is far more qualified to comment on that area of code than I am. However here is what I am seeing:
The file scheme reaches construct_loader with a valid namemap (0x887950) and libctx. So the default provider is active and offering the file store loader
The insert into namemap 0x887950 / hashtable 0x8879a0 succeeds (rc=1). Struct sizes are the expected 32‑bit values (HT_KEY=8 vs 16 on x86‑64), keysize=64, and the hash is computed.
Immediately after the successful insert, in the same namemap, under the same write lock, the lookup returns 0 - not found, i.e. The value goes into the hashtable and cannot be read back out. @nhorman says:
My working assumption has been 3.5.6 because the error output is showing a 3.5.6 path:
It would be good to get this definitively confirmed. I think it would be helpful to do another round of debug logging this time in the hashtable implementation. @uglyoldbob - please can you apply the following and try again: From b4aef073fa4ff2de57f5adab6ba373c52be48052 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.foundation>
Date: Mon, 6 Jul 2026 08:45:57 +0100
Subject: [PATCH] Instrument ossl_ht_insert/ossl_ht_get for the i586 namemap
bug
Trace the store and lookup of the 'file' entry: neigh/slot, hash and
value round-trip on insert, and compare_hash/match_key on get. Gated
on the file hash.
Assisted-by: Claude Code:claude-opus-4-8
---
crypto/hashtable/hashtable.c | 41 ++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 8b9f9cc0d9..b8d26be702 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -50,12 +50,16 @@
* searched for a match until an empty entry is found.
*/
+#include <stdio.h>
#include <string.h>
#include <internal/rcu.h>
#include <internal/hashtable.h>
#include <internal/hashfunc.h>
#include <openssl/rand.h>
+/* Hash of the "file" store scheme; used only to gate debug output */
+#define DBG_FILE_HASH 12890292847699326835ULL
+
/*
* gcc defines __SANITIZE_THREAD__
* but clang uses the feature attributes api
@@ -588,6 +592,21 @@ not_found:
h->wpd.value_count++;
ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[empty_idx].value,
&newval);
+
+ if (hash == DBG_FILE_HASH) {
+ uint64_t rbhash = 0;
+ struct ht_internal_value_st *rbval;
+
+ (void)CRYPTO_atomic_load(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
+ &rbhash, h->atomic_lock);
+ rbval = ossl_rcu_deref(&md->neighborhoods[neigh_idx].entries[empty_idx].value);
+ fprintf(stderr, "DEBUG ossl_ht_insert[file]: stored at neigh=%llu slot=%zu; "
+ "readback hash=%llu (match=%d) value=%p (match=%d) value_count=%zu\n",
+ (unsigned long long)neigh_idx, empty_idx,
+ (unsigned long long)rbhash, rbhash == hash,
+ (void *)rbval, rbval == newval, h->wpd.value_count);
+ }
+
return 1;
}
@@ -675,11 +694,25 @@ HT_VALUE *ossl_ht_get(HT *h, HT_KEY *key)
md = ossl_rcu_deref(&h->md);
neigh_idx = neigh_idx_start = hash & md->neighborhood_mask;
+
+ if (hash == DBG_FILE_HASH)
+ fprintf(stderr, "DEBUG ossl_ht_get[file]: hash=%llu neigh_idx=%llu "
+ "mask=%llu NEIGHBORHOOD_LEN=%zu lockfree8=%d atomic_lock=%p\n",
+ (unsigned long long)hash, (unsigned long long)neigh_idx,
+ (unsigned long long)md->neighborhood_mask, (size_t)NEIGHBORHOOD_LEN,
+ (int)__atomic_is_lock_free(8, &md->neighborhoods[neigh_idx].entries[0].hash),
+ (void *)h->atomic_lock);
+
do {
PREFETCH_NEIGHBORHOOD(md->neighborhoods[neigh_idx]);
for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
ival = ossl_rcu_deref(&md->neighborhoods[neigh_idx].entries[j].value);
if (ival == NULL) {
+ if (hash == DBG_FILE_HASH)
+ fprintf(stderr, "DEBUG ossl_ht_get[file]: neigh=%llu j=%zu EMPTY "
+ "(lockless_reads=%d -> %s)\n",
+ (unsigned long long)neigh_idx, j, lockless_reads,
+ lockless_reads ? "return NULL" : "continue");
if (lockless_reads)
/* lockless_reads implies no deletion, we can break out */
return NULL;
@@ -688,6 +721,11 @@ HT_VALUE *ossl_ht_get(HT *h, HT_KEY *key)
if (!CRYPTO_atomic_load(&md->neighborhoods[neigh_idx].entries[j].hash,
&ehash, h->atomic_lock))
return NULL;
+ if (hash == DBG_FILE_HASH)
+ fprintf(stderr, "DEBUG ossl_ht_get[file]: neigh=%llu j=%zu ehash=%llu "
+ "compare_hash=%d match_key=%d\n",
+ (unsigned long long)neigh_idx, j, (unsigned long long)ehash,
+ compare_hash(hash, ehash), match_key(&ival->value.key, key));
if (compare_hash(hash, ehash) && match_key(&ival->value.key, key))
return (HT_VALUE *)ival;
}
@@ -697,6 +735,9 @@ HT_VALUE *ossl_ht_get(HT *h, HT_KEY *key)
neigh_idx = (neigh_idx + 1) & md->neighborhood_mask;
} while (neigh_idx != neigh_idx_start);
+ if (hash == DBG_FILE_HASH)
+ fprintf(stderr, "DEBUG ossl_ht_get[file]: exhausted search, returning NULL\n");
+
return NULL;
}
--
2.43.0 |
Beta Was this translation helpful? Give feedback.
-
|
I am using openssl 3.5.6. I applied the change for CACHE_LINE_BYTES, but did not see any change in behavior, so I reverted that change. |
Beta Was this translation helpful? Give feedback.
-
This is wrong!! The hash has been corrupted here! This is what the code currently looks like here: if (!CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
hash, h->atomic_lock))
return 0;
h->wpd.value_count++;
ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[empty_idx].value,
&newval);
if (hash == DBG_FILE_HASH) {
uint64_t rbhash = 0;
struct ht_internal_value_st *rbval;
(void)CRYPTO_atomic_load(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
&rbhash, h->atomic_lock);
rbval = ossl_rcu_deref(&md->neighborhoods[neigh_idx].entries[empty_idx].value);
fprintf(stderr, "DEBUG ossl_ht_insert[file]: stored at neigh=%llu slot=%zu; "
"readback hash=%llu (match=%d) value=%p (match=%d) value_count=%zu\n",
(unsigned long long)neigh_idx, empty_idx,
(unsigned long long)rbhash, rbhash == hash,
(void *)rbval, rbval == newval, h->wpd.value_count);So, in the first line, we do an atomic store of the value stored in This actually suggests a corruption somewhere in the int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
{
#if defined(__GNUC__) && defined(__ATOMIC_ACQ_REL) && !defined(BROKEN_CLANG_ATOMICS)
if (__atomic_is_lock_free(sizeof(*dst), dst)) {
__atomic_store(dst, &val, __ATOMIC_RELEASE);
return 1;
}
#elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
/* This will work for all future Solaris versions. */
if (dst != NULL) {
atomic_swap_64(dst, val);
return 1;
}
#endif
if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))
return 0;
*dst = val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
}and int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
{
#if defined(__GNUC__) && defined(__ATOMIC_ACQ_REL) && !defined(BROKEN_CLANG_ATOMICS)
if (__atomic_is_lock_free(sizeof(*val), val)) {
__atomic_load(val, ret, __ATOMIC_ACQUIRE);
return 1;
}
#elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
/* This will work for all future Solaris versions. */
if (ret != NULL) {
*ret = atomic_or_64_nv(val, 0);
return 1;
}
#endif
if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
return 0;
*ret = *val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
}What compiler are you using? Assuming gcc then that means we're into the first I'm wondering whether this is actually a compiler bug! |
Beta Was this translation helpful? Give feedback.
-
|
I using gcc, versio 13.4.0 |
Beta Was this translation helpful? Give feedback.
-
|
Here's a quick standalone C program to see if we can reproduce a compiler bug: #include <stdint.h>
#include <stdio.h>
int main(void)
{
volatile uint64_t slot = 0;
uint64_t in = 0xb2e3850c9de07773ULL, out = 0;
printf("lock_free(8) = %d\n", __atomic_is_lock_free(8, (void *)&slot));
__atomic_store_n(&slot, in, __ATOMIC_RELEASE);
out = __atomic_load_n(&slot, __ATOMIC_ACQUIRE);
printf("in = 0x%016llx\n", (unsigned long long)in);
printf("raw = 0x%016llx (plain read of slot)\n", (unsigned long long)slot);
printf("out = 0x%016llx (__atomic_load)\n", (unsigned long long)out);
printf("match = %d\n", out == in);
return out != in;
}Please check the flags that openssl was compiled with and use the same ones. You can do that with $ openssl version -f
compiler: gcc -fPIC -pthread -m64 -Wa,--noexecstack -Wall -fzero-call-used-regs=used-gpr -DOPENSSL_TLS_SECURITY_LEVEL=2 -Wa,--noexecstack -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/build/openssl-gza1f8/openssl-3.0.13=. -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/build/openssl-gza1f8/openssl-3.0.13=/usr/src/openssl-3.0.13-0ubuntu3.11 -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_BUILDING_OPENSSL -DNDEBUG -Wdate-time -D_FORTIFY_SOURCE=3Copy and paste everything after Probably worth repeating this with |
Beta Was this translation helpful? Give feedback.
-
|
@mattcaswell beat me too it, but I agree, the fact that a store followed immediately by a load of the same address suggests that something is wrong with the atomic operations on this system (either in the openssl functions themselves, which seems sort of unlikely, given how small they are, and the fact that they work everywhere else), or in the atomic operations emitted by the compiler themselves. I built the head of the tree with gcc 16.1.1 here with -m32 -march=i586, and got this output when I disassembled it: That struck me as super odd, given that I'd never seen the fildll or fistpll instructions previously, so I went and looked them up. From https://en.wikipedia.org/wiki/List_of_x86_instructions: FILDLL loads a signed 64 bit value onto the stack, converting to floating point Thats super wierd for a few reasons:
@uglyoldbob I know were asking alot here, but can you run objdump on the threads_pthread object file and take a look to see if your compiler is emitting the same code? If so, I'm starting to wonder if, given that my more recent compiler is emitting these instructions, it wouldn't be better to make i586 builds use the fallback locking scheme rather than atomics here |
Beta Was this translation helpful? Give feedback.
-
|
I compiled and ran the atomic test program. Here is the disassembly of it (with the same gcc compiler used to build openssl 3.5.6) Here is what happens when I run it. And here is some disassembly from build/crypto/libcrypto-lib-threads_pthread.o |
Beta Was this translation helpful? Give feedback.
-
|
I tried compiling with clang, that didn't help so I reverted back to using gcc. |
Beta Was this translation helpful? Give feedback.
-
|
Thats just wild: gcc is using floating point instructions to atomic 64 bit loads/stores. From what I read, its done to avoid having to use the LOCK prefix on instructions, which is much slower. Using -DBROKEN_CLANG_ATOMICS: It makes sense that would work as it backs off everything in the CRYPTO_atomic api to use locks rather than atomic operations. I'm not sure whats going on here exactly, but it definitely seems like something about those instructions use is transiently causing some corruption (maybe a misaligned pointer address? i don't know). But I wonder if the better part of sanity here is to mark i586 in a way that it uses locking rather than atomics entirely. @mattcaswell ? |
Beta Was this translation helpful? Give feedback.
-
|
So, this is definitive. This isn't an OpenSSL bug:
This is entirely unexpected and clearly shows that the "store" operation has failed to store correctly!! It's surprising that gcc uses the FPU (fildll/fistpll) to do a 64-bit atomic at all. But, IIUC, fildll/fistpll is supposed to round-trip a 64-bit integer losslessly, so this really shouldn't corrupt anything. The corruption we are seeing (top 53 bits preserved, bottom 11 bits corrupted) fits with a floating point value and a 53 bit mantissa. So, this is plausibly not even a gcc bug! Could this be hardware specific!!? But this stuff is definitely way outside my area of expertise. @uglyoldbob - are you running this on actual silicon? Or is this some kind of emulated environment?
I'm hesitant to do that. We've not had other reports of this problem. If this was a widespread problem I would have expected someone else to have encountered it before now. My suggestion for now is that @uglyoldbob just uses the -DBROKEN_CLANG_ATOMICS workaround. That's kind of why that define is there - to work around broken atomics. |
Beta Was this translation helpful? Give feedback.
-
|
I am using a qemu emulation before I try to run on actual hardware due to debug limitations on the actual hardware platform. I think using the DBROKEN_CLANG_ATOMICS for my platform is fine for now. |
Beta Was this translation helpful? Give feedback.
-
Ah!! So, in all probability this looks like a bug in QEMU. If you're not running the latest version of QEMU you might consider upgrading. You might also want to raise a bug report with the QEMU team. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
I am using yocto scarthgap to build a distro for an i586 machine (the original xbox to be specific), and have run into an issue regarding the openssl library. On version 3.5.5 and 3.5.6 I am seeing the same behavior.
touch /tmp/asdf && openssl storeutl file:/tmp/asdfThis results in no store loader found. I am using the stock yocto configuration for openssl without customization.
Beta Was this translation helpful? Give feedback.
All reactions