[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Thu Dec 30 11:55:01 UTC 2021


The branch, master has been updated
       via  96b10702295 smbd: Assert we don't leak fd's in struct fd_handle
       via  529e6718c09 smbd: Replace SMB_VFS_CLOSE() calls with fd_close()
       via  e6c8b38ecf1 vfs_commit: Reset fsp->fd->fd to -1 after SMB_VFS_CLOSE
       via  28e09580b05 pysmbd: Fix file descriptor leaks
       via  5988607d7fa smbd: Fix a fd leak when closing a print file
      from  9d2bf015378 s3:libsmb: fix signing regression SMBC_server_internal()

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 96b1070229545a7c7e223dddadb9e8503d7d8b6a
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Dec 27 11:17:22 2021 +0100

    smbd: Assert we don't leak fd's in struct fd_handle
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Thu Dec 30 11:54:17 UTC 2021 on sn-devel-184

commit 529e6718c0944ce2e31ba5c72799bedd8569541c
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 28 12:25:59 2021 +0100

    smbd: Replace SMB_VFS_CLOSE() calls with fd_close()
    
    fd_close() mostly wraps SMB_VFS_CLOSE() but also takes care of refcounting
    fsp->fh properly and also makes sure that fsp->fh->fd is set to -1 after close.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit e6c8b38ecf1f040630a91a859d5f5bf528ceffbd
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 28 18:42:00 2021 +0100

    vfs_commit: Reset fsp->fd->fd to -1 after SMB_VFS_CLOSE
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 28e09580b05951d2c1f5a6c57a1287b51e034e35
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 28 18:34:20 2021 +0100

    pysmbd: Fix file descriptor leaks
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 5988607d7fa3f5f62cf7e0f9517b471c1db19aee
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 28 12:25:40 2021 +0100

    smbd: Fix a fd leak when closing a print file
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/modules/vfs_commit.c |  1 +
 source3/smbd/close.c         |  1 +
 source3/smbd/durable.c       | 48 +++++++++++++++++++-------------------------
 source3/smbd/fd_handle.c     |  9 +++++++++
 source3/smbd/open.c          | 17 ++++++----------
 source3/smbd/pysmbd.c        | 32 +++++++++++++++++++++++------
 source3/torture/cmd_vfs.c    | 19 +++++++++---------
 7 files changed, 74 insertions(+), 53 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/vfs_commit.c b/source3/modules/vfs_commit.c
index a933a5982e5..6d64896c7e0 100644
--- a/source3/modules/vfs_commit.c
+++ b/source3/modules/vfs_commit.c
@@ -244,6 +244,7 @@ static int commit_openat(struct vfs_handle_struct *handle,
 		if (SMB_VFS_FSTAT(fsp, &st) == -1) {
 			int saved_errno = errno;
 			SMB_VFS_CLOSE(fsp);
+			fsp_set_fd(fsp, -1);
 			errno = saved_errno;
                         return -1;
                 }
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 0ea0f096fea..610450d086f 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -1542,6 +1542,7 @@ NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
 	} else if (fsp->print_file != NULL) {
 		/* FIXME: return spool errors */
 		print_spool_end(fsp, close_type);
+		fd_close(fsp);
 		file_free(req, fsp);
 		status = NT_STATUS_OK;
 	} else if (!fsp->fsp_flags.is_fsa) {
diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 88e0b70d137..a49bca6fd61 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -837,15 +837,15 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 
 	ret = SMB_VFS_FSTAT(fsp, &fsp->fsp_name->st);
 	if (ret == -1) {
+		NTSTATUS close_status;
 		status = map_nt_error_from_unix_common(errno);
 		DEBUG(1, ("Unable to fstat stream: %s => %s\n",
 			  smb_fname_str_dbg(smb_fname),
 			  nt_errstr(status)));
-		ret = SMB_VFS_CLOSE(fsp);
-		if (ret == -1) {
-			DEBUG(0, ("vfs_default_durable_reconnect: "
-				  "SMB_VFS_CLOSE failed (%s) - leaking file "
-				  "descriptor\n", strerror(errno)));
+		close_status = fd_close(fsp);
+		if (!NT_STATUS_IS_OK(close_status)) {
+			DBG_ERR("fd_close failed (%s) - leaking file "
+				"descriptor\n", nt_errstr(close_status));
 		}
 		TALLOC_FREE(lck);
 		op->compat = NULL;
@@ -855,11 +855,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 	}
 
 	if (!S_ISREG(fsp->fsp_name->st.st_ex_mode)) {
-		ret = SMB_VFS_CLOSE(fsp);
-		if (ret == -1) {
-			DEBUG(0, ("vfs_default_durable_reconnect: "
-				  "SMB_VFS_CLOSE failed (%s) - leaking file "
-				  "descriptor\n", strerror(errno)));
+		NTSTATUS close_status = fd_close(fsp);
+		if (!NT_STATUS_IS_OK(close_status)) {
+			DBG_ERR("fd_close failed (%s) - leaking file "
+				"descriptor\n", nt_errstr(close_status));
 		}
 		TALLOC_FREE(lck);
 		op->compat = NULL;
@@ -870,11 +869,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 
 	file_id = vfs_file_id_from_sbuf(conn, &fsp->fsp_name->st);
 	if (!file_id_equal(&cookie.id, &file_id)) {
-		ret = SMB_VFS_CLOSE(fsp);
-		if (ret == -1) {
-			DEBUG(0, ("vfs_default_durable_reconnect: "
-				  "SMB_VFS_CLOSE failed (%s) - leaking file "
-				  "descriptor\n", strerror(errno)));
+		NTSTATUS close_status = fd_close(fsp);
+		if (!NT_STATUS_IS_OK(close_status)) {
+			DBG_ERR("fd_close failed (%s) - leaking file "
+				"descriptor\n", nt_errstr(close_status));
 		}
 		TALLOC_FREE(lck);
 		op->compat = NULL;
@@ -889,11 +887,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 						      &fsp->fsp_name->st,
 						      fsp_str_dbg(fsp));
 	if (!ok) {
-		ret = SMB_VFS_CLOSE(fsp);
-		if (ret == -1) {
-			DEBUG(0, ("vfs_default_durable_reconnect: "
-				  "SMB_VFS_CLOSE failed (%s) - leaking file "
-				  "descriptor\n", strerror(errno)));
+		NTSTATUS close_status = fd_close(fsp);
+		if (!NT_STATUS_IS_OK(close_status)) {
+			DBG_ERR("fd_close failed (%s) - leaking file "
+				"descriptor\n", nt_errstr(close_status));
 		}
 		TALLOC_FREE(lck);
 		op->compat = NULL;
@@ -904,13 +901,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 
 	status = set_file_oplock(fsp);
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("vfs_default_durable_reconnect failed to set oplock "
-			  "after opening file: %s\n", nt_errstr(status)));
-		ret = SMB_VFS_CLOSE(fsp);
-		if (ret == -1) {
-			DEBUG(0, ("vfs_default_durable_reconnect: "
-				  "SMB_VFS_CLOSE failed (%s) - leaking file "
-				  "descriptor\n", strerror(errno)));
+		NTSTATUS close_status = fd_close(fsp);
+		if (!NT_STATUS_IS_OK(close_status)) {
+			DBG_ERR("fd_close failed (%s) - leaking file "
+				"descriptor\n", nt_errstr(close_status));
 		}
 		TALLOC_FREE(lck);
 		op->compat = NULL;
diff --git a/source3/smbd/fd_handle.c b/source3/smbd/fd_handle.c
index 766b6c52c13..94d15ef848b 100644
--- a/source3/smbd/fd_handle.c
+++ b/source3/smbd/fd_handle.c
@@ -37,6 +37,12 @@ struct fd_handle {
 	uint64_t gen_id;
 };
 
+static int fd_handle_destructor(struct fd_handle *fh)
+{
+	SMB_ASSERT((fh->fd == -1) || (fh->fd == AT_FDCWD));
+	return 0;
+}
+
 struct fd_handle *fd_handle_create(TALLOC_CTX *mem_ctx)
 {
 	struct fd_handle *fh = NULL;
@@ -46,6 +52,9 @@ struct fd_handle *fd_handle_create(TALLOC_CTX *mem_ctx)
 		return NULL;
 	}
 	fh->fd = -1;
+
+	talloc_set_destructor(fh, fd_handle_destructor);
+
 	return fh;
 }
 
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 7f1aedbd1fb..6a9a1d9a9dc 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -801,10 +801,8 @@ static NTSTATUS non_widelink_open(const struct files_struct *dirfsp,
 		 * and errno=ELOOP.
 		 */
 		if (S_ISLNK(fsp->fsp_name->st.st_ex_mode)) {
-			ret = SMB_VFS_CLOSE(fsp);
-			SMB_ASSERT(ret == 0);
-
-			fsp_set_fd(fsp, -1);
+			status = fd_close(fsp);
+			SMB_ASSERT(NT_STATUS_IS_OK(status));
 			fd = -1;
 			status = NT_STATUS_STOPPED_ON_SYMLINK;
 		}
@@ -1158,7 +1156,6 @@ static NTSTATUS reopen_from_procfd(struct files_struct *fsp,
 	int old_fd;
 	int new_fd;
 	NTSTATUS status;
-	int ret;
 
 	if (!fsp->fsp_flags.have_proc_fds) {
 		return NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -1197,15 +1194,13 @@ static NTSTATUS reopen_from_procfd(struct files_struct *fsp,
 				mode);
 	if (new_fd == -1) {
 		status = map_nt_error_from_unix(errno);
-		SMB_VFS_CLOSE(fsp);
-		fsp_set_fd(fsp, -1);
+		fd_close(fsp);
 		return status;
 	}
 
-	ret = SMB_VFS_CLOSE(fsp);
-	fsp_set_fd(fsp, -1);
-	if (ret != 0) {
-		return map_nt_error_from_unix(errno);
+	status = fd_close(fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
 	fsp_set_fd(fsp, new_fd);
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index e3ed6dba5d5..17a27e8cb35 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -156,6 +156,13 @@ static int set_sys_acl_conn(const char *fname,
 
 	ret = SMB_VFS_SYS_ACL_SET_FD(smb_fname->fsp, acltype, theacl);
 
+	status = fd_close(smb_fname->fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(frame);
+		errno = map_errno_from_nt_status(status);
+		return -1;
+	}
+
 	TALLOC_FREE(frame);
 	return ret;
 }
@@ -275,7 +282,7 @@ static NTSTATUS set_nt_acl_conn(const char *fname,
 		DBG_ERR("init_files_struct failed: %s\n",
 			nt_errstr(status));
 		if (fsp != NULL) {
-			SMB_VFS_CLOSE(fsp);
+			fd_close(fsp);
 		}
 		TALLOC_FREE(frame);
 		return status;
@@ -286,7 +293,7 @@ static NTSTATUS set_nt_acl_conn(const char *fname,
 		DEBUG(0,("set_nt_acl_no_snum: fset_nt_acl returned %s.\n", nt_errstr(status)));
 	}
 
-	SMB_VFS_CLOSE(fsp);
+	fd_close(fsp);
 
 	TALLOC_FREE(frame);
 	return status;
@@ -333,6 +340,12 @@ static NTSTATUS get_nt_acl_conn(TALLOC_CTX *mem_ctx,
 			nt_errstr(status));
 	}
 
+	status = fd_close(smb_fname->fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(frame);
+		return status;
+	}
+
 	TALLOC_FREE(frame);
 
 	return status;
@@ -618,7 +631,7 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 		DBG_ERR("init_files_struct failed: %s\n",
 			nt_errstr(status));
 		if (fsp != NULL) {
-			SMB_VFS_CLOSE(fsp);
+			fd_close(fsp);
 		}
 		TALLOC_FREE(frame);
 		/*
@@ -631,13 +644,13 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 	ret = SMB_VFS_FCHOWN(fsp, uid, gid);
 	if (ret != 0) {
 		int saved_errno = errno;
-		SMB_VFS_CLOSE(fsp);
+		fd_close(fsp);
 		TALLOC_FREE(frame);
 		errno = saved_errno;
 		return PyErr_SetFromErrno(PyExc_OSError);
 	}
 
-	SMB_VFS_CLOSE(fsp);
+	fd_close(fsp);
 	TALLOC_FREE(frame);
 
 	Py_RETURN_NONE;
@@ -1042,6 +1055,13 @@ static PyObject *py_smbd_get_sys_acl(PyObject *self, PyObject *args, PyObject *k
 		return PyErr_SetFromErrno(PyExc_OSError);
 	}
 
+	status = fd_close(smb_fname->fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(frame);
+		PyErr_SetNTSTATUS(status);
+		return NULL;
+	}
+
 	py_acl = py_return_ndr_struct("samba.dcerpc.smb_acl", "t", acl, acl);
 
 	TALLOC_FREE(frame);
@@ -1210,7 +1230,7 @@ static PyObject *py_smbd_create_file(PyObject *self, PyObject *args, PyObject *k
 		DBG_ERR("init_files_struct failed: %s\n",
 			nt_errstr(status));
 	} else if (fsp != NULL) {
-		SMB_VFS_CLOSE(fsp);
+		fd_close(fsp);
 	}
 
 	TALLOC_FREE(frame);
diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c
index a2bece1f5dd..72fa784d72b 100644
--- a/source3/torture/cmd_vfs.c
+++ b/source3/torture/cmd_vfs.c
@@ -429,7 +429,7 @@ static NTSTATUS cmd_open(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, c
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
-		SMB_VFS_CLOSE(fsp);
+		fd_close(fsp);
 		TALLOC_FREE(fsp);
 		TALLOC_FREE(smb_fname);
 		return status;
@@ -513,7 +513,8 @@ static NTSTATUS cmd_pathfunc(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int arg
 
 static NTSTATUS cmd_close(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, const char **argv)
 {
-	int fd, ret;
+	int fd;
+	NTSTATUS status;
 
 	if (argc != 2) {
 		printf("Usage: close <fd>\n");
@@ -526,15 +527,15 @@ static NTSTATUS cmd_close(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 		return NT_STATUS_OK;
 	}
 
-	ret = SMB_VFS_CLOSE(vfs->files[fd]);
-	if (ret == -1 )
-		printf("close: error=%d (%s)\n", errno, strerror(errno));
+	status = fd_close(vfs->files[fd]);
+	if (!NT_STATUS_IS_OK(status))
+		printf("close: error=%s\n", nt_errstr(status));
 	else
 		printf("close: ok\n");
 
 	TALLOC_FREE(vfs->files[fd]);
 	vfs->files[fd] = NULL;
-	return NT_STATUS_OK;
+	return status;
 }
 
 
@@ -1816,9 +1817,9 @@ static NTSTATUS cmd_set_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int a
 out:
 	TALLOC_FREE(sd);
 
-	ret = SMB_VFS_CLOSE(fsp);
-	if (ret == -1 )
-		printf("close: error=%d (%s)\n", errno, strerror(errno));
+	status = fd_close(fsp);
+	if (!NT_STATUS_IS_OK(status))
+		printf("close: error= (%s)\n", nt_errstr(status));
 
 	TALLOC_FREE(fsp);
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list