open
https://gitlab.synchro.net/main/sbbs/-/work_items/1335
`get_sess_list_entry()` in `src/sbbs3/ssl.c` takes the `cert_epoch_lock` read lock and never releases it. Both places that should unlock call `rwlock_rdlock()` again; the log text beside each says "unlock":
```c
if (!rwlock_rdlock(&cert_epoch_lock)) {
lprintf(LOG_ERR, "Failed to lock cert_epoch_lock for read at %d", __LINE__);
return NULL;
}
assert_pthread_mutex_lock(&ssl_cert_list_mutex);
while (1) {
if (cert_list == NULL) {
assert_pthread_mutex_unlock(&ssl_cert_list_mutex);
if (!rwlock_rdlock(&cert_epoch_lock)) { // <-- should be rwlock_unlock()
lprintf(LOG_ERR, "Failed to unlock cert_epoch_lock for read at %d", __LINE__);
}
return get_ssl_cert(cfg, lprintf);
}
...
}
assert_pthread_mutex_unlock(&ssl_cert_list_mutex);
if (!rwlock_rdlock(&cert_epoch_lock)) { // <-- should be rwlock_unlock()
lprintf(LOG_ERR, "Failed to unlock cert_epoch_lock for read at %d", __LINE__);
}
return ret;
```
Every `add_private_key()` call therefore leaves two read locks held by the calling thread, for the life of the process. Introduced in c07616e78b (2024-01-19, "Clean up and push down locks.").
### Why it has not hung anything
The only writer is `ssl_new_epoch()`, and it is unreachable on a system that already has an `ssl.cert`:
- `tls_cert_file_date` starts at 0 and is assigned only inside `ssl_new_epoch()`.
- `ssl_sync()` calls `ssl_new_epoch()` only `if (epoch_date != 0)`.
- The other caller is `create_self_signed_cert()`.
So unless a self-signed certificate is created in the running process, `tls_cert_file_date` stays 0 and no new epoch ever starts.
### Consequences
1. **A replaced `ssl.cert` is never detected.** A renewed certificate (e.g. from letsyncrypt) is not picked up by `ssl_sync()` at server recycle; the cached contexts in `cert_list` keep serving the old one until the process restarts.
2. **Fixing (1) alone exposes the leak.** Once `ssl_new_epoch()` can run, `rwlock_wrlock(&cert_epoch_lock)` waits forever on the leaked readers (`while (lock->readers)` in the Win32 `rwlock_wrlock()`; the same with pthread rwlocks), and with a writer waiting, new readers block too, so every new TLS session hangs.
3. On the `cert_list == NULL` path the thread still holds its read locks when it calls `get_ssl_cert()` → `ssl_sync()` → `ssl_new_epoch()`, so a write lock taken there would self-deadlock even after the two `rdlock` calls become unlocks, unless the unlock happens before `get_ssl_cert()` (which is where the second call sits).
The self-signed path looks exposed today, by reading: with no `ssl.cert` and `create_self_signed_cert` enabled, the first TLS session goes `get_sess_list_entry()` (two read locks held) → `get_ssl_cert()` → `create_self_signed_cert()` → `ssl_new_epoch()` → `rwlock_wrlock(&cert_epoch_lock)` on the same thread. The certificate file has been written by then, so a restart would clear it.
All of this is from reading the code while investigating unrelated errors; none of it has been observed as a hang or reproduced.
— *Authored by Claude (Claude Code), on behalf of @rswindell*
--- SBBSecho 3.38-Linux
* Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)