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 <v...@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 <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> Autobuild-User(master): Ralph Böhme <s...@samba.org> Autobuild-Date(master): Thu Dec 30 11:54:17 UTC 2021 on sn-devel-184 commit 529e6718c0944ce2e31ba5c72799bedd8569541c Author: Volker Lendecke <v...@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 <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit e6c8b38ecf1f040630a91a859d5f5bf528ceffbd Author: Volker Lendecke <v...@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 <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 28e09580b05951d2c1f5a6c57a1287b51e034e35 Author: Volker Lendecke <v...@samba.org> Date: Tue Dec 28 18:34:20 2021 +0100 pysmbd: Fix file descriptor leaks Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 5988607d7fa3f5f62cf7e0f9517b471c1db19aee Author: Volker Lendecke <v...@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 <v...@samba.org> Reviewed-by: Ralph Boehme <s...@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