Fix use-after-free in Connection.read_fls binding (keep_alive) - #71
Open
z0mz0m wants to merge 1 commit into
Open
Fix use-after-free in Connection.read_fls binding (keep_alive)#71z0mz0m wants to merge 1 commit into
z0mz0m wants to merge 1 commit into
Conversation
TableReader stores 'Connection& m_connection' (table_reader.hpp), but
the binding returned it with return_value_policy::move and no lifetime
tie. The Connection is therefore free to be collected while the reader
is still alive, and the reader then dereferences freed memory.
This crashes the documented one-liner from README.md and
examples/python_example.py:
pyfastlanes.connect().read_fls('data.fls').to_csv('decoded.csv')
lldb on the unfixed build:
EXC_BAD_ACCESS (code=1, address=0x6574617669727077)
frame #0: fastlanes::Rowgroup::Rowgroup(RowgroupDescriptorT const&,
Connection const&)
(the faulting address is ASCII text — freed std::string bytes read as a
pointer). It reproduces on the bundled data/example corpus.
py::keep_alive<0, 1>() makes the returned reader keep its Connection
alive, which is the invariant the C++ type already assumes. Verified:
the README chain, an explicitly deleted connection followed by
gc.collect(), and a reader returned from a function whose local
connection went out of scope all now succeed and produce byte-identical
output to the pattern that previously worked by accident.
Co-Authored-By: Claude Fable 5 <[email protected]>
z0mz0m
added a commit
to Zetta-Zip/FastLanes
that referenced
this pull request
Jul 27, 2026
…reader TableReader holds Connection& , but the binding returned it with return_value_policy::move and no lifetime tie, so the documented connect().read_fls(p).to_csv(q) chain dereferenced freed memory. Upstream PR: cwida#71 Co-Authored-By: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TableReaderstores a reference to its connection:but the binding returns it with
return_value_policy::moveand no lifetime tie, so nothing keeps theConnectionalive for as long as the reader. The connection can be collected while the reader is still in use, after which the reader dereferences freed memory.This crashes the one-liner documented in
README.mdand used inexamples/python_example.py:Under lldb on an unfixed build:
The faulting address is ASCII text — freed
std::stringbytes being read as a pointer. It reproduces on the bundleddata/examplecorpus, so it is not data-dependent.py::keep_alive<0, 1>()makes the returned reader keep itsConnectionalive, which is the invariant the C++ type already assumes.Verified on macOS 15 / arm64 (Apple clang 21). Three patterns that all crashed before and all pass after, each producing byte-identical output to the pattern that previously worked by accident (holding the connection in a local):
reader = conn.read_fls(p); del conn; gc.collect(); reader.to_csv(q);Note this branch alone will not compile under clang 21 without #70 (unrelated
-Wnonnullfailure inTypedStats); the two changes are independent in content but not in build order.🤖 Generated with Claude Code