Skip to content

LMDB support is broken #2601

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
hyc opened this issue Aug 9, 2021 · 3 comments
Closed

LMDB support is broken #2601

hyc opened this issue Aug 9, 2021 · 3 comments

Comments

@hyc
Copy link
Contributor

hyc commented Aug 9, 2021

lmdb.cc misuses the LMDB API in multiple ways

The code calls mdb_dbi_open/mdb_dbi_close in multiple locations without any concurrency protection.
The API doc clearly states that mdb_dbi_open must not be called from multiple transactions at once.
The doc also clearly states that mdb_dbi_close is not protected at all and must never be called from
more than one thread at once. The doc also states that programs SHOULD only call mdb_dbi_open
once, when the environment is created, and should only call mdb_dbi_close when the env is being
destroyed.

http://www.lmdb.tech/doc/group__mdb.html#ga52dd98d0c542378370cd6b712ff961b5
http://www.lmdb.tech/doc/group__mdb.html#ga52dd98d0c542378370cd6b712ff961b5

The code calls mdb_txn_abort on invalid transactions after mdb_txn|_commit fails. The doc clearly
states that after mdb_txn_commt has been called, the txn handle is freed. As such, passing the handle
the txn_abort references freed memory and triggers undefined behavior.

http://www.lmdb.tech/doc/group__mdb.html#ga846fbd6f46105617ac9f4d76476f6597

Logs and dumps

A crash caused by these bugs has been reported here https://bugs.openldap.org/show_bug.cgi?id=9626

@martinhsv
Copy link
Contributor

martinhsv commented Sep 2, 2021

Hi @hyc ,

Thank you for the detailed report. It does indeed look like we have at least some issues there.

I'll try to take a closer look at this soon.

martinhsv added a commit that referenced this issue Feb 9, 2022
@martinhsv
Copy link
Contributor

@hyc ,

Thanks again for this. It was much appreciated.

@ziollek
Copy link
Contributor

ziollek commented Mar 6, 2022

@hyc - thanks for Your changes. I use intensively lmdb with modsecurity and faced significant performance issues caused by misusing write transactions instead of read-only.
Unfortunately during testing Your changes it turns out that i cannot read collections via read-only transactions from lmdb because MDB_env is opened too soon (in nginx master before forking). I prepared another PR which fixed that issue - i hope it helps another lmdb users ;)

@martinhsv - i believe that fix will be very helpful and should also be added to 3.0.7 release.

vladbukin pushed a commit to vladbukin/ModSecurity that referenced this issue Apr 12, 2022
Only open DBI once, doesn't need closing.
Never reuse a txn handle after commit.
Use MDB_RDONLY for txns that aren't doing any writes
daixiang0 pushed a commit to daixiang0/ModSecurity that referenced this issue Apr 24, 2022
Only open DBI once, doesn't need closing.
Never reuse a txn handle after commit.
Use MDB_RDONLY for txns that aren't doing any writes
leyao-daily pushed a commit to leyao-daily/ModSecurity that referenced this issue Jul 26, 2022
Only open DBI once, doesn't need closing.
Never reuse a txn handle after commit.
Use MDB_RDONLY for txns that aren't doing any writes
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants