Skip to content

Commit

Permalink
Fix use_after_free bug when underlying FS enables kFSBuffer (#11645)
Browse files Browse the repository at this point in the history
Summary:
Fix use_after_free bug in async_io MultiReads when underlying FS enabled kFSBuffer. kFSBuffer is when underlying FS pass their own buffer instead of using RocksDB scratch in FSReadRequest
Since it's an experimental feature, added a hack for now to fix the bug.
Planning to make public API change to remove const from the callback as it doesn't make sense to use const.

Pull Request resolved: #11645

Test Plan: tested locally

Reviewed By: ltamasi

Differential Revision: D47819907

Pulled By: akankshamahajan15

fbshipit-source-id: 1faf5ef795bf27e2b3a60960374d91274931df8d
  • Loading branch information
akankshamahajan15 authored and facebook-github-bot committed Jul 27, 2023
1 parent c24ef26 commit 63a5125
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
1 change: 1 addition & 0 deletions unreleased_history/bug_fixes/fsbuffer_bug_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix use_after_free bug in async_io MultiReads when underlying FS enabled kFSBuffer. kFSBuffer is when underlying FS pass their own buffer instead of using RocksDB scratch in FSReadRequest. Right now it's an experimental feature.
5 changes: 5 additions & 0 deletions util/async_file_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ bool AsyncFileReader::MultiReadAsyncImpl(ReadAwaiter* awaiter) {
FSReadRequest* read_req = static_cast<FSReadRequest*>(cb_arg);
read_req->status = req.status;
read_req->result = req.result;
if (req.fs_scratch != nullptr) {
// TODO akanksha: Revisit to remove the const in the callback.
FSReadRequest& req_tmp = const_cast<FSReadRequest&>(req);
read_req->fs_scratch = std::move(req_tmp.fs_scratch);
}
},
&awaiter->read_reqs_[i], &awaiter->io_handle_[i], &awaiter->del_fn_[i],
/*aligned_buf=*/nullptr);
Expand Down

0 comments on commit 63a5125

Please sign in to comment.