]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
vfs_glusterfs: fix directory fd leak via FSP extension destructor master
authorThales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
Fri, 27 Feb 2026 07:38:06 +0000 (04:38 -0300)
committerGünther Deschner <gd@samba.org>
Wed, 8 Apr 2026 16:46:12 +0000 (16:46 +0000)
When Samba closes a directory backed by vfs_glusterfs, the glfs_fd_t
opened by vfs_gluster_openat() is never closed.  This leaks one
libgfapi file descriptor and one server-side fd_t in glusterfsd per
directory open/close cycle.  With persistent SMB2 connections the
leak is unbounded and drives monotonic RSS growth on the GlusterFS
brick process.

The leak happens because vfs_glusterfs creates two independent
glfs_fd_t handles per directory: one via glfs_open() in
vfs_gluster_openat(), stored in the FSP extension, and another via
glfs_opendir() in vfs_gluster_fdopendir(), tracked by struct smb_Dir.
On close, smb_Dir_destructor() closes the opendir handle and sets the
pathref fd to -1.  fd_close() then returns early without calling
SMB_VFS_CLOSE, so vfs_gluster_close() never runs and the glfs_open()
handle is orphaned.  The original code passed NULL as the destroy
callback to VFS_ADD_FSP_EXTENSION, so there was no safety net.

The default VFS does not have this problem because fdopendir(3) wraps
the existing kernel fd rather than opening a new handle.  libgfapi
has no equivalent -- glfs_opendir() always creates an independent
handle by path.  The actual glfs_fd_t is stored in the FSP extension,
not in fsp->fh->fd (which holds a sentinel value), so Samba's generic
close path cannot reach it.

Register vfs_gluster_fsp_ext_destroy() as the FSP extension destroy
callback.  It calls glfs_close() on the stored pointer and is invoked
by vfs_remove_all_fsp_extensions() during file_free(), which runs
unconditionally for every fsp.  In the explicit close path,
vfs_gluster_close() NULLs the extension pointer before calling
VFS_REMOVE_FSP_EXTENSION to prevent double-close.  This follows the
same pattern used by vfs_ceph_new.c (vfs_ceph_fsp_ext_destroy_cb).

Observed on a production file server with persistent SMB2 connections
and continuous directory operations.  GlusterFS brick statedumps
showed fd_t pool growth from 1,993 to 80,350 active instances over
6 days, roughly 13,000 leaked fds per day per brick.

RN: Fix a directory file descriptor leak in vfs_glusterfs that caused
unbounded memory growth on the GlusterFS brick with persistent SMB2
connections.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=16043

Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
Reviewed-by: Anoop C S <anoopcs@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Wed Apr  8 16:46:12 UTC 2026 on atb-devel-224

source3/modules/vfs_glusterfs.c

index 4f02164103efe1c38f5ad31ec3ba85c5f6823a8c..d61d5521c70c5ee51525cb1620b4614c2f881af6 100644 (file)
@@ -614,6 +614,24 @@ static uint32_t vfs_gluster_fs_capabilities(struct vfs_handle_struct *handle,
        return caps;
 }
 
+/*
+ * FSP extension destructor -- ensures the glfs_fd_t opened by
+ * vfs_gluster_openat() is closed even when vfs_gluster_close() is
+ * never reached.  This happens for directory FSPs: fd_close() calls
+ * dptr_CloseDir(), whose talloc destructor smb_Dir_destructor() calls
+ * fsp_set_fd(fsp, -1).  fd_close() then sees fsp_get_pathref_fd() == -1
+ * and returns without calling SMB_VFS_CLOSE().  The destructor is
+ * invoked by vfs_remove_all_fsp_extensions() during file_free().
+ */
+static void vfs_gluster_fsp_ext_destroy(void *p_data)
+{
+       glfs_fd_t **glfdp = (glfs_fd_t **)p_data;
+       if (glfdp != NULL && *glfdp != NULL) {
+               glfs_close(*glfdp);
+               *glfdp = NULL;
+       }
+}
+
 static glfs_fd_t *vfs_gluster_fetch_glfd(struct vfs_handle_struct *handle,
                                         const files_struct *fsp)
 {
@@ -750,7 +768,8 @@ static int vfs_gluster_openat(struct vfs_handle_struct *handle,
                return -1;
        }
 
-       p_tmp = VFS_ADD_FSP_EXTENSION(handle, fsp, glfs_fd_t *, NULL);
+       p_tmp = VFS_ADD_FSP_EXTENSION(handle, fsp, glfs_fd_t *,
+                                     vfs_gluster_fsp_ext_destroy);
        if (p_tmp == NULL) {
                END_PROFILE(syscall_openat);
                errno = ENOMEM;
@@ -830,7 +849,11 @@ static int vfs_gluster_openat(struct vfs_handle_struct *handle,
 
        if (glfd == NULL) {
                END_PROFILE(syscall_openat);
-               /* no extension destroy_fn, so no need to save errno */
+               /*
+                * glfd pointer is still NULL (zero-initialized by
+                * VFS_ADD_FSP_EXTENSION), so the destroy callback
+                * is a no-op and errno is preserved.
+                */
                VFS_REMOVE_FSP_EXTENSION(handle, fsp);
                return -1;
        }
@@ -846,22 +869,21 @@ static int vfs_gluster_close(struct vfs_handle_struct *handle,
                             files_struct *fsp)
 {
        int ret;
-       glfs_fd_t *glfd = NULL;
+       glfs_fd_t **glfd = (glfs_fd_t **)VFS_FETCH_FSP_EXTENSION(handle, fsp);
 
        START_PROFILE(syscall_close);
 
-       glfd = vfs_gluster_fetch_glfd(handle, fsp);
-       if (glfd == NULL) {
+       if (glfd == NULL || *glfd == NULL) {
                END_PROFILE(syscall_close);
                DBG_ERR("Failed to fetch gluster fd\n");
                return -1;
        }
 
+       ret = glfs_close(*glfd);
+       *glfd = NULL;  /* Prevent double close in fsp destructor */
        VFS_REMOVE_FSP_EXTENSION(handle, fsp);
 
-       ret = glfs_close(glfd);
        END_PROFILE(syscall_close);
-
        return ret;
 }