Skip to content

Commit

Permalink
fix(prefix): fix bug when buffer realloc occurs during prefix lookup
Browse files Browse the repository at this point in the history
Prefix based lookup may need to reallocate the key/value buffer when the
buffer size exceeds the default (currently 1MB).

However, because we use the `ops[0]` and `ops[1]` passed in is used for both
providing the `after` and `prefix`, in case of `NGX_AGAIN`, these will
be overwritten by the `ngx_lua_resty_lmdb_ffi_prefix()` function to the
first and second key respectively, which breaks the retry and causing
the second call to `ngx_lua_resty_lmdb_ffi_prefix()` to return empty.

This commit fixes the above issue by always resetting `ops[0]` and
`ops[1]` between retries, therefore the above issue no longer occurs.

Also, this commit fixes a use-after-free bug for `db_drop()` operation when
map is full.
  • Loading branch information
dndx committed Nov 21, 2024
1 parent 890b3ca commit b6574dc
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 9 deletions.
16 changes: 8 additions & 8 deletions lib/resty/lmdb/prefix.lua
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ function _M.page(start, prefix, db, page_size)
local value_buf_size = get_string_buf_size()
local ops = ffi_new("ngx_lua_resty_lmdb_operation_t[?]", page_size)

ops[0].opcode = C.NGX_LMDB_OP_PREFIX
ops[0].key.data = start
ops[0].key.len = #start

ops[1].opcode = C.NGX_LMDB_OP_PREFIX
ops[1].key.data = prefix
ops[1].key.len = #prefix

local dbi, err = get_dbi(false, db or DEFAULT_DB)
if err then
return nil, "unable to open DB for access: " .. err
Expand All @@ -55,6 +47,14 @@ function _M.page(start, prefix, db, page_size)
ops[0].dbi = dbi

::again::
ops[0].opcode = C.NGX_LMDB_OP_PREFIX
ops[0].key.data = start
ops[0].key.len = #start

ops[1].opcode = C.NGX_LMDB_OP_PREFIX
ops[1].key.data = prefix
ops[1].key.len = #prefix

local buf = get_string_buf(value_buf_size, false)
local ret = C.ngx_lua_resty_lmdb_ffi_prefix(ops, page_size,
buf, value_buf_size, err_ptr)
Expand Down
3 changes: 2 additions & 1 deletion src/ngx_lua_resty_lmdb_transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ int ngx_lua_resty_lmdb_ffi_execute(ngx_lua_resty_lmdb_operation_t *ops,
rc = mdb_txn_commit(txn);
if (rc != 0) {
*err = mdb_strerror(rc);
goto err;

return NGX_ERROR;
}

} else {
Expand Down
75 changes: 75 additions & 0 deletions t/10-prefix.t
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,78 @@ false.../lua-resty-lmdb/lua-resty-lmdb/lib/resty/lmdb/prefix.lua:34: 'page_size'
[error]
[warn]
[crit]



=== TEST 9: prefix.page() operation with large page size [KAG-5874]
--- http_config eval: $::HttpConfig
--- main_config eval: $::MainConfig
--- config
location = /t {
content_by_lua_block {
local l = require("resty.lmdb")
ngx.say(l.db_drop(true))
ngx.say(l.set("test", "value"))
local inserted = { test = "value" }
for i = 1, 2048 do
-- string.rep with 120 makes sure each page goes just over the 1MB
-- default buffer size and triggers a realloc
assert(l.set(string.rep("test", 120) .. i, string.rep("value", 120)))
inserted[string.rep("test", 120) .. i] = string.rep("value", 120)
end
ngx.say(l.set("u", "value4"))
ngx.say(l.set("u1", "value5"))
local p = require("resty.lmdb.prefix")
local res, err = p.page("test", "test", nil, 1000)
if not res then
ngx.say("page errored: ", err)
end
ngx.say("FIRST PAGE")
for _, pair in ipairs(res) do
inserted[pair.key] = nil
end
res, err = p.page(res[#res].key .. "\x00", "test", nil, 1000)
if not res then
ngx.say("page errored: ", err)
end
ngx.say("SECOND PAGE")
for _i, pair in ipairs(res) do
inserted[pair.key] = nil
end
res, err = p.page(res[#res].key .. "\x00", "test", nil, 1000)
if not res then
ngx.say("page errored: ", err)
end
ngx.say("THIRD PAGE")
for _i, pair in ipairs(res) do
inserted[pair.key] = nil
end
ngx.say(next(inserted))
}
}
--- request
GET /t
--- response_body
true
true
true
true
FIRST PAGE
SECOND PAGE
THIRD PAGE
nil
--- no_error_log
[error]
[warn]
[crit]

0 comments on commit b6574dc

Please sign in to comment.