Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

LMDB support is broken #2601

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
Projects

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 martinhsv added this to Backog in v3.0.7 Jan 10, 2022
@martinhsv martinhsv moved this from Backlog to In progress in v3.0.7 Jan 27, 2022
martinhsv added a commit that referenced this issue Feb 9, 2022
@martinhsv martinhsv moved this from In progress to Done in v3.0.7 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

3 participants