Skip to content

Commit 6dc7756

Browse files
committed
client: fix memory leak in Client::CRF_iofinish::complete
Commit 1210ddf ("Client: Add non-blocking helper classes") introduced Client::C_Read_Finisher Context object for async READ operations, but it has a read-after-free bug which may cause memory leak when calling libcephf's non-blocking ceph_ll_nonblocking_readv_writev API with async READ: ceph_ll_nonblocking_readv_writev (READ) Client::ll_preadv_pwritev ... Client::_read_async Context::complete Client::CRF_iofinish::complete Client::CRF_iofinish::finish CRF->finish_io() Client::C_Read_Finisher::finish_io ... delete this; // frees CRF_iofinish->CRF if (CRF->iofinished) // use-after-free of CRF delete this; // may not get here A possible memory leak depends on timing and race with other thread allocation which alters the memory address of CRF->iofinished to false, thus skipping the last delete operation. The check of `if (CRF->iofinished)` is unnecessary: it is always set to true upon calling CRF->finish_io(). Thus, there is no need to have the override function Client::CRF_iofinish::complete() as it now has the same logic as Context::complete(). Removed. Signed-off-by: Shachar Sharon <[email protected]>
1 parent d28e5fe commit 6dc7756

File tree

1 file changed

+0
-8
lines changed

1 file changed

+0
-8
lines changed

src/client/Client.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,14 +1389,6 @@ class Client : public Dispatcher, public md_config_obs_t {
13891389
void finish(int r) override {
13901390
CRF->finish_io(r);
13911391
}
1392-
1393-
// For _read_async, we may not finish in one go, so be prepared for multiple
1394-
// calls to complete. All the handling though is in C_Read_Finisher.
1395-
void complete(int r) override {
1396-
finish(r);
1397-
if (CRF->iofinished)
1398-
delete this;
1399-
}
14001392
};
14011393

14021394
class C_Read_Sync_NonBlocking : public Context {

0 commit comments

Comments
 (0)