From b5bd466cc8d59eff302bf96ac8dd2d404548e07c Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 30 May 2025 17:01:48 +0800 Subject: [PATCH] rgw/driver/posix: closedir() to free dir handle Previously, we had Resource leak in Directory::for_each() where directory streams opened with fdopendir() were never properly closed, causing a 32KB leak per unclosed directory handle. In this change, we add closedir() call to properly release directory stream resources after enumeration completes. ASan report: ``` Direct leak of 32816 byte(s) in 1 object(s) allocated from: #0 0x738387320e15 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67 #1 0x738381383514 (/usr/lib/libc.so.6+0xe3514) (BuildId: 468e3585c794491a48ea75fceb9e4d6b1464fc35) #2 0x738381383418 in fdopendir (/usr/lib/libc.so.6+0xe3418) (BuildId: 468e3585c794491a48ea75fceb9e4d6b1464fc35) #3 0x6433e1aefac4 in for_each > /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:874 #4 0x6433e1aa10ab in rgw::sal::Directory::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function, fu2::abi_310::detail::property > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:1077 #5 0x6433e1aa8974 in rgw::sal::MPDirectory::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function, fu2::abi_310::detail::property > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:1384 #6 0x6433e1abf1ee in rgw::sal::POSIXBucket::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function, fu2::abi_310::detail::property > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:2236 #7 0x6433e1c43956 in file::listing::BucketCache::fill(DoutPrefixProvider const*, file::listing::BucketCacheEntry*, rgw::sal::POSIXBucket*, unsigned int, optional_yield) /home/kef u/dev/ceph/src/rgw/driver/posix/bucket_cache.h:368 #8 0x6433e1c1c71c in file::listing::BucketCache::list_bucket(DoutPrefixProvider const*, optional_yield, rgw::sal::POSIXBucket*, std::__cxx11::basic_string, std::allocator >, fu2::abi_310::d etail::function, fu2::abi_310::detail::property >) /home/kefu/dev/ceph/src/rgw/driver/posix/bucket_cache.h:410 #9 0x6433e1ac1829 in rgw::sal::POSIXBucket::list(DoutPrefixProvider const*, rgw::sal::Bucket::ListParams&, int, rgw::sal::Bucket::ListResults&, optional_yield) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:2256 #10 0x6433e1ae1302 in rgw::sal::POSIXMultipartUpload::list_parts(DoutPrefixProvider const*, ceph::common::CephContext*, int, int, int*, bool*, optional_yield, bool) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:3692 #11 0x6433e1ae2f3b in rgw::sal::POSIXMultipartUpload::complete(DoutPrefixProvider const*, optional_yield, ceph::common::CephContext*, std::map, std::allocator >, std::less, std::allocator, std::allocator > > > >&, std::__cxx11::list >&, unsigned long&, bool&, RGWCompressionInfo&, long&, std::__cxx11::basic_string, std::allocator >&, ACLOwner&, unsigned long, rgw::sal::Object*, boost::container::flat_map, std::allocator >, std::less, std::allocator > >, void>, std::less, void>&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:3770 #12 0x6433e0fffb73 in POSIXMPObjectTest::create_MPObj(rgw::sal::MultipartUpload*, std::__cxx11::basic_string, std::allocator >) /home/kefu/dev/ceph/src/test/rgw/test_rgw_posix_driver.cc:1717 #13 0x6433e0e95ab9 in POSIXMPObjectTest_BucketList_Test::TestBody() /home/kefu/dev/ceph/src/test/rgw/test_rgw_posix_driver.cc:1863 #14 0x6433e1308d17 in void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/kefu/dev/ceph/src/googletest/googletest/src/gtest.cc:2653 ``` Fixes https://tracker.ceph.com/issues/71505 Signed-off-by: Kefu Chai --- src/rgw/driver/posix/rgw_sal_posix.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/rgw/driver/posix/rgw_sal_posix.cc b/src/rgw/driver/posix/rgw_sal_posix.cc index df625c8c078..9bd71a4ee35 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -351,6 +351,7 @@ static int delete_directory(int parent_fd, const char* dname, bool delete_childr return -ret; } } + closedir(dir); ret = unlinkat(parent_fd, dname, AT_REMOVEDIR); if (ret < 0) { @@ -899,6 +900,13 @@ int Directory::for_each(const DoutPrefixProvider* dpp, const F& func) /* Limit reached */ ret = 0; } + + closedir(dir); + // closedir() closes the fd, so we need to invalidate it + fd = -1; + // closedir() closes fd, but succeeding calls might assume that fd is still valid. + // so let's reopen it. + open(dpp); return ret; } -- 2.39.5