From 6dc77563d4dac8c7e2f41dae445acba7694fa192 Mon Sep 17 00:00:00 2001 From: Shachar Sharon Date: Tue, 22 Oct 2024 15:06:54 +0300 Subject: [PATCH] client: fix memory leak in Client::CRF_iofinish::complete Commit 1210ddf7a ("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 --- src/client/Client.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/client/Client.h b/src/client/Client.h index 08485c844c47a..e1ee5714ef0c9 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -1389,14 +1389,6 @@ private: void finish(int r) override { CRF->finish_io(r); } - - // For _read_async, we may not finish in one go, so be prepared for multiple - // calls to complete. All the handling though is in C_Read_Finisher. - void complete(int r) override { - finish(r); - if (CRF->iofinished) - delete this; - } }; class C_Read_Sync_NonBlocking : public Context { -- 2.39.5