Fix `MDB_BAD_RSLOT` in `subaddress_reader::update_reader()` causing permanently missed subaddress outputs (#278)
What the developer wrote
Authored by Evgeny
Fix `MDB_BAD_RSLOT` in `subaddress_reader::update_reader()` causing permanently missed subaddress outputs (#278)
* fix: regtest
* fix: MDB_BAD_RSLOT
What changed, and why it matters
This commit fixes a database-handling bug in Monero Light Wallet Server (LWS). The bug caused the server to permanently miss some subaddress outputs because a read transaction was not closed before a new one was opened, triggering an LMDB error (MDB_BAD_RSLOT). The fix explicitly releases the old read transaction before starting the next one. There is no direct evidence this is exploitable by an attacker; it appears to be a reliability/correctness bug.
Apply the patch. Monitor for any remaining LMDB transaction-lifetime issues in `subaddress_reader` and related readers. Consider adding regression tests that exercise repeated `update_reader()` calls under LMDB default settings.
Security signals we found
Fixes LMDB transaction-slot error (MDB_BAD_RSLOT)
Prevents permanently missed subaddress outputs
Changes test expectations to reflect recovered output
Adds regtest flag propagation in server startup
Evidence from the diff
In subaddress_reader::update_reader(), the code previously assigned reader = disk.start_read() while the existing reader (an LMDB read transaction) was still active on the same thread. Under LMDB’s default reader-slot tracking (non-MDB_NOTLS), this produced MDB_BAD_RSLOT, causing subaddress output lookups to fail silently and outputs to be missed permanently. The patch resets reader to an invalid state first, releasing the old transaction, then starts a new read transaction. The commit also threads a regtest flag into scanner.sync() and updates unit-test expectations to account for one additional output and an expanded subaddress range.
Changed components
src/util/ownership_test.cppsrc/server_main.cpptests/unit/scanner.test.cppInspect captured patch +8 / −3
### src/server_main.cpp
@@ -387,7 +387,7 @@ namespace
mempool = std::make_shared<lws::mempool>();
}
- auto client = scanner.sync(ctx.connect().value(), prog.untrusted_daemon).value();
+ auto client = scanner.sync(ctx.connect().value(), prog.untrusted_daemon, prog.regtest).value();
lws::rest_server server{
epee::to_span(prog.rest_servers),
### src/util/ownership_test.cpp
@@ -305,6 +305,11 @@ namespace lws
void subaddress_reader::update_reader()
{
+ // Release the current read txn before opening the next one - `reader = disk.start_read()`
+ // would otherwise begin the new LMDB read txn (inside start_read()) while the old one
+ // is still open on this thread, which LMDB's default (non-MDB_NOTLS) reader-slot tracking
+ // does not support and reports as MDB_BAD_RSLOT.
+ reader = expect<db::storage_reader>{common_error::kInvalidArgument};
reader = disk.start_read();
if (!reader)
MERROR("Subadress lookup failure: " << reader.error().message());
### tests/unit/scanner.test.cpp
@@ -941,7 +941,7 @@ LWS_CASE("lws::scanner::sync and lws::scanner::run")
auto reader = MONERO_UNWRAP(db.start_read());
auto outputs = MONERO_UNWRAP(reader.get_outputs(lws::db::account_id(1)));
- EXPECT(outputs.count() == 6);
+ EXPECT(outputs.count() == 7);
auto output_it = outputs.make_iterator();
for (auto output_it = outputs.make_iterator(); !output_it.is_end(); ++output_it)
{
@@ -1000,7 +1000,7 @@ LWS_CASE("lws::scanner::sync and lws::scanner::run")
{
const std::vector<lws::db::subaddress_dict> expected_range{
- {lws::db::major_index(0), {{lws::db::index_range{lws::db::minor_index(0), lws::db::minor_index(2)}}}}
+ {lws::db::major_index(0), {{lws::db::index_range{lws::db::minor_index(0), lws::db::minor_index(3)}}}}
};
EXPECT(MONERO_UNWRAP(reader.get_subaddresses(lws::db::account_id(1))) == expected_range);
}Why this scored 56/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.