The branch, master has been updated
via 434e6d4b4b4 smbd: Only file_free() a self-created fsp in
create_file_unixpath()
via e91b59c4dfb smbd: Introduce close_file_smb()
via e751c6237b7 smbd: Factor out fsp_unbind_smb() from file_free()
via 5f1ceead709 torture: Add a test to show that full_audit uses a ptr
after free
via 93fe9c83145 smbd: Simplify the flow in close_file_free()
via 1fbd9877fea smbd: No base fsps to close_file_free() from
file_close_user()
via 61f57ba24ee smbd: Factor out close_file_in_loop() from
file_close_conn_fn()
via d1341d666af smbd: No base fsps to close_file_free() from
file_close_conn()
via f5bc73a2ad9 smbd: NULL out "fsp" in close_file()
via 363ac753389 smbd: Call file_free() just once in close_file()
via 244c5a7d31c smbd: Move the call to file_free() out of
close_fake_file()
via 2293ca5b572 smbd: Move the call to file_free() out of
close_normal_file()
via 9966b5e233e smbd: Move the call to file_free() out of
close_directory()
via 1c1734974fc smbd: Slightly simplify create_file_unixpath()
from cd06574b528 s3:winbind: Reduce the level and improve a couple of
debug messages
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 434e6d4b4b45757878642d229d26d146792a3878
Author: Volker Lendecke <v...@samba.org>
Date: Thu Feb 3 17:17:07 2022 +0100
smbd: Only file_free() a self-created fsp in create_file_unixpath()
This fixes a use-after-free in smb_full_audit_create_file() when
calling SMB_VFS_CREATE_FILE with fsp->fsp_name as smb_fname.
create_file_unixpath() has this comment:
* This is really subtle. If someone passes in an smb_fname
* where smb_fname actually is taken from fsp->fsp_name, then
* the lifetime of these objects is meant to be the same.
so it seems legitimate to call CREATE_FILE this way.
When CREATE_FILE runs into an error, create_file_unixpath() does a
file_free, which also takes fsp->fsp_name with
it. smb_full_audit_create_file() wants to log the failure including
the smb_fname after NEXT_CREATE_FILE has exited, but this will then
use the already free'ed data.
Fix by only doing the file_free() on an fsp that
create_file_unixpath() created itself.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Thu Feb 10 19:11:33 UTC 2022 on sn-devel-184
commit e91b59c4dfb2b35661dbecbc5769584109e23571
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 9 18:03:33 2022 +0100
smbd: Introduce close_file_smb()
This does almost everything that close_file_free() does, but it leaves
the fsp around.
A normal close_file() now calls fsp_unbind_smb() twice. Functionally
this is not a problem, fsp_unbind_smb() is idempotent. The only
potential performance penalty might come from the loops in
remove_smb2_chained_fsp(), but those only are potentially large with
deeply queued smb2 requests. If that turns out to be a problem, we'll
cope with it later. The alternative would be to split up file_free()
into even more routines and make it more difficult to figure out which
of the "rundown/unbind/free" routines to call in any particular
situation.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit e751c6237b750adb4cb59df4a42bb9f39354e7e4
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 9 17:23:03 2022 +0100
smbd: Factor out fsp_unbind_smb() from file_free()
For example, remove our entry from smbXsrv_open_global.tdb
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 5f1ceead7094aefc6ad1f209468e9ea8f009716c
Author: Volker Lendecke <v...@samba.org>
Date: Thu Feb 3 15:25:11 2022 +0100
torture: Add a test to show that full_audit uses a ptr after free
Run vfstest with this vfstest.cmd under valgrind and you'll see what
happens. Exact explanation a few patches further down...
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 93fe9c83145d31ea11a9cd25049ac527ad4a000d
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 2 12:42:08 2022 +0100
smbd: Simplify the flow in close_file_free()
We are no longer called on base_fsp's in SHUTDOWN_CLOSE. That
simplifies the logic in the common case, we now have a linear flow for
the very often-called close_file()
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 1fbd9877fead466a17d697c143cd370c0b27f610
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 2 08:58:15 2022 +0100
smbd: No base fsps to close_file_free() from file_close_user()
Same logic as the change for file_close_conn()
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 61f57ba24ee2e54abf224118f93bd0ccda44ec41
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 2 12:27:50 2022 +0100
smbd: Factor out close_file_in_loop() from file_close_conn_fn()
To be reused in file_close_user(). Deliberately a separate commit to
make the previous commit easier to understand.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit d1341d666af12965b4318f89b1d0e1e8769e861e
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 2 08:58:15 2022 +0100
smbd: No base fsps to close_file_free() from file_close_conn()
close_file_free() needs to handle base fsps specially. This can be
simplified a lot if we pass the the open files a second time in case
we encountered base_fsps that we could not immediately delete.
file_close_conn() is not our hot code path, and also we don't expect
many thousand open files that we need to walk a second time.
A subsequent patch will simplify close_file_free(), the complicated
logic is now in files.c, where it IMHO belongs because
file_set_base_fsp() are here as well.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit f5bc73a2ad97647f76143f7962c964f45aa6b1a0
Author: Volker Lendecke <v...@samba.org>
Date: Tue Feb 1 17:47:29 2022 +0100
smbd: NULL out "fsp" in close_file()
Quite a few places already had this in the caller, but not all. Rename
close_file() to close_file_free() appropriately. We'll factor out
close_file_smb() doing only parts of close_file_free() later.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 363ac7533895fda786f56c4fe8346128753f38a5
Author: Volker Lendecke <v...@samba.org>
Date: Tue Feb 1 17:21:24 2022 +0100
smbd: Call file_free() just once in close_file()
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 244c5a7d31c3a37082b320680f2b71108d77bbd4
Author: Volker Lendecke <v...@samba.org>
Date: Tue Feb 1 17:19:54 2022 +0100
smbd: Move the call to file_free() out of close_fake_file()
Centralize calling file_free(), but leave close_fake_file() in for API
symmetry reasons.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 2293ca5b572178404273856f8d8989a5ee7de80c
Author: Volker Lendecke <v...@samba.org>
Date: Tue Feb 1 17:17:36 2022 +0100
smbd: Move the call to file_free() out of close_normal_file()
Call file_free() just once
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 9966b5e233ef2ff0368ba5860c824c7cd6420415
Author: Volker Lendecke <v...@samba.org>
Date: Tue Feb 1 17:14:34 2022 +0100
smbd: Move the call to file_free() out of close_directory()
Call file_free() just once
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 1c1734974fcf1d060bc6bcdbe1858cba1b7e5a73
Author: Volker Lendecke <v...@samba.org>
Date: Wed Feb 9 10:02:46 2022 +0100
smbd: Slightly simplify create_file_unixpath()
Avoid the "needs_fsp_unlink" variable, describe the talloc hierarchy a
bit differently in the comments.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/lib/adouble.c | 20 +--
source3/modules/vfs_fruit.c | 12 +-
source3/modules/vfs_worm.c | 2 +-
source3/printing/nt_printing.c | 10 +-
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 4 +-
source3/script/tests/full_audit_segfault/run.sh | 23 ++++
.../script/tests/full_audit_segfault/vfstest.cmd | 3 +
source3/selftest/tests.py | 8 ++
source3/smbd/close.c | 104 ++++++--------
source3/smbd/dir.c | 9 +-
source3/smbd/fake_file.c | 4 +-
source3/smbd/files.c | 150 +++++++++++++++++----
source3/smbd/nttrans.c | 6 +-
source3/smbd/open.c | 75 +++++------
source3/smbd/proto.h | 9 +-
source3/smbd/reply.c | 62 ++++-----
source3/smbd/smb2_close.c | 2 +-
source3/smbd/smb2_create.c | 3 +-
source3/smbd/trans2.c | 48 +++----
source3/torture/cmd_vfs.c | 85 ++++++++++++
source3/utils/net_vfs.c | 7 +-
21 files changed, 415 insertions(+), 231 deletions(-)
create mode 100755 source3/script/tests/full_audit_segfault/run.sh
create mode 100644 source3/script/tests/full_audit_segfault/vfstest.cmd
Changeset truncated at 500 lines:
diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
index 37fb686f17b..aa78007dadd 100644
--- a/source3/lib/adouble.c
+++ b/source3/lib/adouble.c
@@ -1254,13 +1254,13 @@ static bool ad_convert_xattr(vfs_handle_struct *handle,
if (nwritten == -1) {
DBG_ERR("SMB_VFS_PWRITE failed\n");
saved_errno = errno;
- close_file(NULL, fsp, ERROR_CLOSE);
+ close_file_free(NULL, &fsp, ERROR_CLOSE);
errno = saved_errno;
ok = false;
goto fail;
}
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
ok = false;
goto fail;
@@ -1395,12 +1395,12 @@ static bool ad_convert_finderinfo(vfs_handle_struct
*handle,
if (nwritten == -1) {
DBG_ERR("SMB_VFS_PWRITE failed\n");
saved_errno = errno;
- close_file(NULL, fsp, ERROR_CLOSE);
+ close_file_free(NULL, &fsp, ERROR_CLOSE);
errno = saved_errno;
return false;
}
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
return false;
}
@@ -1652,7 +1652,7 @@ static bool ad_unconvert_open_ad(TALLOC_CTX *mem_ctx,
if (ret != 0) {
DBG_ERR("SMB_VFS_FCHOWN [%s] failed: %s\n",
fsp_str_dbg(fsp), nt_errstr(status));
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
return false;
}
}
@@ -1710,14 +1710,14 @@ static bool ad_unconvert_get_streams(struct
vfs_handle_struct *handle,
num_streams,
streams);
if (!NT_STATUS_IS_OK(status)) {
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
DBG_ERR("streaminfo on [%s] failed: %s\n",
smb_fname_str_dbg(smb_fname),
nt_errstr(status));
return false;
}
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("close_file [%s] failed: %s\n",
smb_fname_str_dbg(smb_fname),
@@ -1975,7 +1975,7 @@ static bool ad_collect_one_stream(struct
vfs_handle_struct *handle,
out:
TALLOC_FREE(sname);
if (fsp != NULL) {
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("close_file [%s] failed: %s\n",
smb_fname_str_dbg(smb_fname),
@@ -2117,9 +2117,9 @@ bool ad_unconvert(TALLOC_CTX *mem_ctx,
out:
if (fsp != NULL) {
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
- DBG_ERR("close_file [%s] failed: %s\n",
+ DBG_ERR("close_file_free() [%s] failed: %s\n",
smb_fname_str_dbg(smb_fname),
nt_errstr(status));
ok = false;
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index d6aa7e3644e..303df41258e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1002,7 +1002,7 @@ static bool readdir_attr_meta_finderi_stream(
fail:
if (fsp != NULL) {
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
return ok;
@@ -4247,8 +4247,8 @@ fail:
DEBUG(10, ("fruit_create_file: %s\n", nt_errstr(status)));
if (fsp) {
- close_file(req, fsp, ERROR_CLOSE);
- *result = fsp = NULL;
+ close_file_free(req, &fsp, ERROR_CLOSE);
+ *result = NULL;
}
return status;
@@ -4993,8 +4993,7 @@ static bool fruit_get_bandsize(vfs_handle_struct *handle,
}
- status = close_file(NULL, fsp, NORMAL_CLOSE);
- fsp = NULL;
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("close_file failed: %s\n", nt_errstr(status));
ok = false;
@@ -5028,11 +5027,10 @@ static bool fruit_get_bandsize(vfs_handle_struct
*handle,
out:
if (fsp != NULL) {
- status = close_file(NULL, fsp, NORMAL_CLOSE);
+ status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("close_file failed: %s\n", nt_errstr(status));
}
- fsp = NULL;
}
TALLOC_FREE(plist);
TALLOC_FREE(smb_fname);
diff --git a/source3/modules/vfs_worm.c b/source3/modules/vfs_worm.c
index 3ae1f9f39e6..76762e0a84f 100644
--- a/source3/modules/vfs_worm.c
+++ b/source3/modules/vfs_worm.c
@@ -75,7 +75,7 @@ static NTSTATUS vfs_worm_create_file(vfs_handle_struct
*handle,
* Access via MAXIMUM_ALLOWED_ACCESS?
*/
if (readonly && ((*result)->access_mask & write_access_flags)) {
- close_file(req, *result, NORMAL_CLOSE);
+ close_file_free(req, result, NORMAL_CLOSE);
return NT_STATUS_ACCESS_DENIED;
}
return NT_STATUS_OK;
diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index a47afda4a84..1e35e017fb2 100644
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -874,8 +874,7 @@ static int file_version_is_newer(connection_struct *conn,
fstring new_file, fstr
(long)old_create_time));
}
- close_file(NULL, fsp, NORMAL_CLOSE);
- fsp = NULL;
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
/* Get file version info (if available) for new file */
status = driver_unix_convert(conn, new_file, &smb_fname);
@@ -935,8 +934,7 @@ static int file_version_is_newer(connection_struct *conn,
fstring new_file, fstr
(long)new_create_time));
}
- close_file(NULL, fsp, NORMAL_CLOSE);
- fsp = NULL;
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
if (use_version && (new_major != old_major || new_minor != old_minor)) {
/* Compare versions and choose the larger version number */
@@ -969,7 +967,7 @@ static int file_version_is_newer(connection_struct *conn,
fstring new_file, fstr
error_exit:
if(fsp)
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
ret = -1;
done:
TALLOC_FREE(smb_fname);
@@ -1177,7 +1175,7 @@ static uint32_t get_correct_cversion(const struct
auth_session_info *session_inf
unbecome_user_without_service();
error_free_conn:
if (fsp != NULL) {
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
if (!W_ERROR_IS_OK(*perr)) {
cversion = -1;
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index 770e5d368a8..ea296eaa6ab 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -2539,7 +2539,7 @@ WERROR _srvsvc_NetGetFileSecurity(struct pipes_struct *p,
error_exit:
if (fsp) {
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
TALLOC_FREE(frame);
@@ -2659,7 +2659,7 @@ WERROR _srvsvc_NetSetFileSecurity(struct pipes_struct *p,
error_exit:
if (fsp) {
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
TALLOC_FREE(frame);
diff --git a/source3/script/tests/full_audit_segfault/run.sh
b/source3/script/tests/full_audit_segfault/run.sh
new file mode 100755
index 00000000000..752b27125c8
--- /dev/null
+++ b/source3/script/tests/full_audit_segfault/run.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+if [ $# -lt 1 ]; then
+cat <<EOF
+Usage: run.sh VFSTEST
+EOF
+exit 1;
+fi
+
+TALLOC_FILL_FREE=0; export TALLOC_FILL_FREE
+
+TESTBASE="$(dirname $0)"
+VFSTEST="$VALGRIND $1"; shift 1;
+ADDARGS="$*"
+
+incdir=`dirname $0`/../../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+testit "vfstest" "$VFSTEST" -f "$TESTBASE/vfstest.cmd" "$ADDARGS" ||
+ failed=$(expr $failed + 1)
+
+exit $failed
diff --git a/source3/script/tests/full_audit_segfault/vfstest.cmd
b/source3/script/tests/full_audit_segfault/vfstest.cmd
new file mode 100644
index 00000000000..84e93e2b157
--- /dev/null
+++ b/source3/script/tests/full_audit_segfault/vfstest.cmd
@@ -0,0 +1,3 @@
+load full_audit
+connect
+create_file .
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 08c8d48ddd5..cab64969491 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -359,6 +359,14 @@ plantestsuite("samba.vfstest.stream_depot",
"nt4_dc:local", [os.path.join(samba3
plantestsuite("samba.vfstest.xattr-tdb-1", "nt4_dc:local",
[os.path.join(samba3srcdir, "script/tests/xattr-tdb-1/run.sh"),
binpath("vfstest"), "$PREFIX", configuration])
plantestsuite("samba.vfstest.acl", "nt4_dc:local", [os.path.join(samba3srcdir,
"script/tests/vfstest-acl/run.sh"), binpath("vfstest"), "$PREFIX",
configuration])
plantestsuite("samba.vfstest.catia", "nt4_dc:local",
[os.path.join(samba3srcdir, "script/tests/vfstest-catia/run.sh"),
binpath("vfstest"), "$PREFIX", configuration])
+plantestsuite(
+ "samba.vfstest.full_audit_segfault",
+ "nt4_dc:local",
+ [os.path.join(samba3srcdir,
+ "script/tests/full_audit_segfault/run.sh"),
+ binpath("vfstest"),
+ "$PREFIX",
+ configuration])
plantestsuite("samba3.blackbox.smbclient_basic.NT1", "nt4_dc_schannel",
[os.path.join(samba3srcdir, "script/tests/test_smbclient_basic.sh"), '$SERVER',
'$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration,
"-mNT1"])
plantestsuite("samba3.blackbox.smbclient_basic.NT1", "nt4_dc_smb1",
[os.path.join(samba3srcdir, "script/tests/test_smbclient_basic.sh"), '$SERVER',
'$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration,
"-mNT1"])
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 8abd3fb3861..206515202e0 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -782,7 +782,6 @@ static NTSTATUS close_normal_file(struct smb_request *req,
files_struct *fsp,
DEBUG(10, ("%s disconnected durable handle for file %s\n",
conn->session_info->unix_info->unix_name,
fsp_str_dbg(fsp)));
- file_free(req, fsp);
return NT_STATUS_OK;
}
@@ -833,7 +832,6 @@ static NTSTATUS close_normal_file(struct smb_request *req,
files_struct *fsp,
conn->num_files_open - 1,
nt_errstr(status) ));
- file_free(req, fsp);
return status;
}
/****************************************************************************
@@ -1379,7 +1377,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
if (lck == NULL) {
DEBUG(0, ("close_directory: Could not get share mode lock for "
"%s\n", fsp_str_dbg(fsp)));
- file_free(req, fsp);
return NT_STATUS_INVALID_PARAMETER;
}
@@ -1429,7 +1426,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(5, ("delete_all_streams failed: %s\n",
nt_errstr(status)));
- file_free(req, fsp);
/* unbecome user. */
pop_sec_ctx();
return status;
@@ -1472,11 +1468,6 @@ static NTSTATUS close_directory(struct smb_request *req,
files_struct *fsp,
strerror(errno)));
}
- /*
- * Do the code common to files and directories.
- */
- file_free(req, fsp);
-
if (NT_STATUS_IS_OK(status) && !NT_STATUS_IS_OK(status1)) {
status = status1;
}
@@ -1484,15 +1475,14 @@ static NTSTATUS close_directory(struct smb_request
*req, files_struct *fsp,
}
/****************************************************************************
- Close a files_struct.
+ Rundown all SMB-related dependencies of a files struct
****************************************************************************/
-NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
- enum file_close_type close_type)
+NTSTATUS close_file_smb(struct smb_request *req,
+ struct files_struct *fsp,
+ enum file_close_type close_type)
{
NTSTATUS status;
- struct files_struct *base_fsp = fsp->base_fsp;
- bool close_base_fsp = false;
/*
* This fsp can never be an internal dirfsp. They must
@@ -1500,44 +1490,10 @@ NTSTATUS close_file(struct smb_request *req,
files_struct *fsp,
*/
SMB_ASSERT(!fsp->fsp_flags.is_dirfsp);
- if (fsp->stream_fsp != NULL) {
- /*
- * fsp is the base for a stream.
- *
- * We're called with SHUTDOWN_CLOSE from files.c which walks the
- * complete list of files.
- *
- * We need to wait until the stream is closed.
- */
- SMB_ASSERT(close_type == SHUTDOWN_CLOSE);
- return NT_STATUS_OK;
- }
-
- if (base_fsp != NULL) {
- /*
- * We need to remove the link in order to
- * recurse for the base fsp below.
- */
- SMB_ASSERT(base_fsp->base_fsp == NULL);
- SMB_ASSERT(base_fsp->stream_fsp == fsp);
- base_fsp->stream_fsp = NULL;
-
- if (close_type == SHUTDOWN_CLOSE) {
- /*
- * We're called with SHUTDOWN_CLOSE from files.c
- * which walks the complete list of files.
- *
- * We may need to defer the SHUTDOWN_CLOSE
- * if it's the next in the linked list.
- *
- * So we only close if the base is *not* the
- * next in the list.
- */
- close_base_fsp = (fsp->next != base_fsp);
- } else {
- close_base_fsp = true;
- }
- }
+ /*
+ * Never call directly on a base fsp
+ */
+ SMB_ASSERT(fsp->stream_fsp == NULL);
if (fsp->fake_file_handle != NULL) {
status = close_fake_file(req, fsp);
@@ -1545,7 +1501,6 @@ NTSTATUS close_file(struct smb_request *req, files_struct
*fsp,
/* 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) {
if (close_type == NORMAL_CLOSE) {
@@ -1558,7 +1513,6 @@ NTSTATUS close_file(struct smb_request *req, files_struct
*fsp,
}
SMB_ASSERT(close_type != NORMAL_CLOSE);
fd_close(fsp);
- file_free(req, fsp);
status = NT_STATUS_OK;
} else if (fsp->fsp_flags.is_directory) {
status = close_directory(req, fsp, close_type);
@@ -1566,21 +1520,43 @@ NTSTATUS close_file(struct smb_request *req,
files_struct *fsp,
status = close_normal_file(req, fsp, close_type);
}
- if (close_base_fsp) {
+ if (fsp->base_fsp != NULL) {
+ /*
+ * fsp was a stream, its base_fsp can't be a stream
+ * as well
+ */
+ SMB_ASSERT(fsp->base_fsp->base_fsp == NULL);
+
+ /*
+ * There's a 1:1 relationship between fsp and a base_fsp
+ */
+ SMB_ASSERT(fsp->base_fsp->stream_fsp == fsp);
/*
- * fsp was a stream, the base fsp can't be a stream as well
- *
- * For SHUTDOWN_CLOSE this is not possible here
- * (if the base_fsp was the next in the linked list), because
- * SHUTDOWN_CLOSE only happens from files.c which walks the
- * complete list of files. If we mess with more than one fsp
- * those loops will become confused.
+ * Make base_fsp look standalone now
*/
+ fsp->base_fsp->stream_fsp = NULL;
- close_file(req, base_fsp, close_type);
+ close_file_free(req, &fsp->base_fsp, close_type);
}
+ fsp_unbind_smb(req, fsp);
+
+ return status;
+}
+
+NTSTATUS close_file_free(struct smb_request *req,
+ struct files_struct **_fsp,
+ enum file_close_type close_type)
+{
+ struct files_struct *fsp = *_fsp;
+ NTSTATUS status;
+
+ status = close_file_smb(req, fsp, close_type);
+
+ file_free(req, fsp);
+ *_fsp = NULL;
+
return status;
}
@@ -1619,5 +1595,5 @@ void msg_close_file(struct messaging_context *msg_ctx,
DEBUG(10,("msg_close_file: failed to find file.\n"));
return;
}
- close_file(NULL, fsp, NORMAL_CLOSE);
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 8e5ce66c961..581ce5202ed 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -185,9 +185,12 @@ void dptr_closecnum(connection_struct *conn)
for(dptr = sconn->searches.dirptrs; dptr; dptr = next) {
next = dptr->next;
if (dptr->conn == conn) {
- files_struct *fsp = dptr->dir_hnd->fsp;
- close_file(NULL, fsp, NORMAL_CLOSE);
- fsp = NULL;
+ /*
+ * Need to make a copy, "dptr" will be gone
+ * after close_file_free() returns
+ */
+ struct files_struct *fsp = dptr->dir_hnd->fsp;
+ close_file_free(NULL, &fsp, NORMAL_CLOSE);
}
}
}
diff --git a/source3/smbd/fake_file.c b/source3/smbd/fake_file.c
index 92ea14a76da..5d669bfe8e5 100644
--- a/source3/smbd/fake_file.c
+++ b/source3/smbd/fake_file.c
@@ -206,6 +206,8 @@ NTSTATUS open_fake_file(struct smb_request *req,
connection_struct *conn,
NTSTATUS close_fake_file(struct smb_request *req, files_struct *fsp)
{
- file_free(req, fsp);
+ /*
+ * Nothing to do, fake files don't hold any resources
+ */
return NT_STATUS_OK;
}
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 4113779f963..3fc1992ce4d 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -759,26 +759,97 @@ NTSTATUS parent_pathref(TALLOC_CTX *mem_ctx,
return NT_STATUS_OK;
}
+static bool close_file_in_loop(struct files_struct *fsp)
+{
--
Samba Shared Repository
