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
