]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
client: fix leak of file handles
authorXavi Hernandez <xhernandez@gmail.com>
Fri, 16 Feb 2024 18:14:07 +0000 (19:14 +0100)
committerXavi Hernandez <xhernandez@gmail.com>
Mon, 11 Mar 2024 14:18:47 +0000 (15:18 +0100)
Based on posix specification, the fd passed to fdopendir() will be
closed by closedir(). However CephFS client wasn't doing that. If the
user opened a directory using ceph_openat(), for example, and then
passed the returned fd to ceph_fdopendir(), the created Fh associated
with the new open was never destroyed.

This patch records the fd used in ceph_fdopendir() so that it can be
closed when ceph_closedir() is called.

Fixes: https://tracker.ceph.com/issues/64479
Signed-off-by: Xavi Hernandez <xhernandez@gmail.com>
(cherry picked from commit fe5c13d2e207473d48aa818484584d67dc3d23b5)

src/client/Client.cc
src/client/Client.h

index 42ca406f84c31e790f38acfa43abd0413004b292..2e23dbf7915a63da24f39ef6ad0d65d74549bd21 100644 (file)
@@ -257,10 +257,10 @@ int Client::get_fd_inode(int fd, InodeRef *in) {
   return r;
 }
 
-dir_result_t::dir_result_t(Inode *in, const UserPerm& perms)
+dir_result_t::dir_result_t(Inode *in, const UserPerm& perms, int fd)
   : inode(in), offset(0), next_offset(2),
     release_count(0), ordered_count(0), cache_index(0), start_shared_gen(0),
-    perms(perms)
+    perms(perms), fd(fd)
   { }
 
 void Client::_reset_faked_inos()
@@ -9014,7 +9014,9 @@ int Client::fdopendir(int dirfd, dir_result_t **dirpp, const UserPerm &perms) {
       return r;
     }
   }
-  r = _opendir(dirinode.get(), dirpp, perms);
+  // Posix says that closedir will also close the file descriptor passed to fdopendir, so we associate
+  // dirfd to the new dir_result_t so that it can be closed later.
+  r = _opendir(dirinode.get(), dirpp, perms, dirfd);
   /* if ENOTDIR, dirpp will be an uninitialized point and it's very dangerous to access its value */
   if (r != -CEPHFS_ENOTDIR) {
       tout(cct) << (uintptr_t)*dirpp << std::endl;
@@ -9022,11 +9024,11 @@ int Client::fdopendir(int dirfd, dir_result_t **dirpp, const UserPerm &perms) {
   return r;
 }
 
-int Client::_opendir(Inode *in, dir_result_t **dirpp, const UserPerm& perms)
+int Client::_opendir(Inode *in, dir_result_t **dirpp, const UserPerm& perms, int fd)
 {
   if (!in->is_dir())
     return -CEPHFS_ENOTDIR;
-  *dirpp = new dir_result_t(in, perms);
+  *dirpp = new dir_result_t(in, perms, fd);
   opened_dirs.insert(*dirpp);
   ldout(cct, 8) << __func__ << "(" << in->ino << ") = " << 0 << " (" << *dirpp << ")" << dendl;
   return 0;
@@ -9054,6 +9056,12 @@ void Client::_closedir(dir_result_t *dirp)
   }
   _readdir_drop_dirp_buffer(dirp);
   opened_dirs.erase(dirp);
+
+  /* Close the associated fd if this dir_result_t comes from an fdopendir request. */
+  if (dirp->fd >= 0) {
+    _close(dirp->fd);
+  }
+
   delete dirp;
 }
 
index 911a8b460dfa443de42b6b4ee03944fe183e9ac7..87593007fff8b1e0330fff8d792d424effd2622e 100644 (file)
@@ -161,7 +161,7 @@ struct dir_result_t {
   };
 
 
-  explicit dir_result_t(Inode *in, const UserPerm& perms);
+  explicit dir_result_t(Inode *in, const UserPerm& perms, int fd);
 
 
   static uint64_t make_fpos(unsigned h, unsigned l, bool hash) {
@@ -238,6 +238,8 @@ struct dir_result_t {
 
   std::vector<dentry> buffer;
   struct dirent de;
+
+  int fd;                // fd attached using fdopendir (-1 if none)
 };
 
 class Client : public Dispatcher, public md_config_obs_t {
@@ -1296,7 +1298,7 @@ private:
 
   void fill_dirent(struct dirent *de, const char *name, int type, uint64_t ino, loff_t next_off);
 
-  int _opendir(Inode *in, dir_result_t **dirpp, const UserPerm& perms);
+  int _opendir(Inode *in, dir_result_t **dirpp, const UserPerm& perms, int fd = -1);
   void _readdir_drop_dirp_buffer(dir_result_t *dirp);
   bool _readdir_have_frag(dir_result_t *dirp);
   void _readdir_next_frag(dir_result_t *dirp);