Skip to content

Commit 785d73d

Browse files
maleadtclaude
andauthored
Avoid use-after-free aborting a failed mdb_txn_commit in DiskCache. (#227)
put_raw! and delete_batch! had a finally block that called mdb_txn_abort whenever the commit hadn't reached the `committed = true` line. But mdb_txn_commit frees the txn handle on both success and failure (per lmdb.h), so when check() threw on a non-zero status the finally aborted an already-freed pointer. Mark the txn as handed off immediately after the commit ccall returns, before letting check() throw. Same bug pattern as JuliaLang/julia#61527 commit 71db484e. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f8f3b21 commit 785d73d

1 file changed

Lines changed: 16 additions & 10 deletions

File tree

src/cache.jl

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ function put_raw!(cache::Cache, key::Vector{UInt8}, framed::Vector{UInt8})
404404
"mdb_txn_begin (write)")
405405
txn = txn_ref[]
406406

407-
committed = false
407+
handed_off = false
408408
try
409409
key_val = Ref(MDB_val(Csize_t(length(key)), pointer(key)))
410410
val_val = Ref(MDB_val(Csize_t(length(framed)), pointer(framed)))
@@ -415,11 +415,14 @@ function put_raw!(cache::Cache, key::Vector{UInt8}, framed::Vector{UInt8})
415415
txn, cache.dbi, key_val, val_val, Cuint(0)) # plain overwrite
416416
check(ret, "mdb_put")
417417

418-
check(ccall((:mdb_txn_commit, liblmdb), Cint, (Ptr{Cvoid},), txn),
419-
"mdb_txn_commit (write)")
420-
committed = true
418+
# mdb_txn_commit frees the txn handle on both success and failure
419+
# (per lmdb.h). Mark as handed off *before* check() can throw, so
420+
# the finally block doesn't abort an already-freed pointer.
421+
ret = ccall((:mdb_txn_commit, liblmdb), Cint, (Ptr{Cvoid},), txn)
422+
handed_off = true
423+
check(ret, "mdb_txn_commit (write)")
421424
finally
422-
committed || ccall((:mdb_txn_abort, liblmdb), Cvoid, (Ptr{Cvoid},), txn)
425+
handed_off || ccall((:mdb_txn_abort, liblmdb), Cvoid, (Ptr{Cvoid},), txn)
423426
end
424427
return
425428
end
@@ -547,7 +550,7 @@ function delete_batch!(cache::Cache, keys::Vector{Vector{UInt8}})
547550
txn = txn_ref[]
548551

549552
deleted = 0
550-
committed = false
553+
handed_off = false
551554
try
552555
for key in keys
553556
key_val = Ref(MDB_val(Csize_t(length(key)), pointer(key)))
@@ -563,11 +566,14 @@ function delete_batch!(cache::Cache, keys::Vector{Vector{UInt8}})
563566
end
564567
end
565568

566-
check(ccall((:mdb_txn_commit, liblmdb), Cint, (Ptr{Cvoid},), txn),
567-
"mdb_txn_commit (evict)")
568-
committed = true
569+
# mdb_txn_commit frees the txn handle on both success and failure
570+
# (per lmdb.h). Mark as handed off *before* check() can throw, so
571+
# the finally block doesn't abort an already-freed pointer.
572+
ret = ccall((:mdb_txn_commit, liblmdb), Cint, (Ptr{Cvoid},), txn)
573+
handed_off = true
574+
check(ret, "mdb_txn_commit (evict)")
569575
finally
570-
committed || ccall((:mdb_txn_abort, liblmdb), Cvoid, (Ptr{Cvoid},), txn)
576+
handed_off || ccall((:mdb_txn_abort, liblmdb), Cvoid, (Ptr{Cvoid},), txn)
571577
end
572578
return deleted
573579
end

0 commit comments

Comments
 (0)