[SCM] Samba Shared Repository - branch master updated
Ralph Böhme
slow at samba.org
Wed Mar 30 15:08:01 UTC 2022
The branch, master has been updated
via 06bfac2125d s3: smbd: Preserve the fsp->fsp_name->st buf across a MSG_SMB_FILE_RENAME message.
via 5e1aa469ae6 s3: smbd: Preserve the fsp->fsp_name->st bufs across rename_open_files()
via 1301e646139 s4: torture: Add test_smb2_close_full_information() test to smb2.rename.
via 4725ef5c963 s4: torture: Add CHECK_CREATED macro to smb2/rename.c. Not yet used.
via e862a2d9ec4 s4: torture: Add CHECK_VAL macro to smb2/rename.c. Not yet used.
via e01c5992b06 s3: tests.py: Only run smb2.rename against fileserver.
from f7f65ceb46d s4:dsdb/descriptor: skip duplicates in descriptor_sd_propagation_object()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 06bfac2125da5e4d37a596d1213912f0c698e69e
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:39:55 2022 -0700
s3: smbd: Preserve the fsp->fsp_name->st buf across a MSG_SMB_FILE_RENAME message.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
Autobuild-User(master): Ralph Böhme <slow at samba.org>
Autobuild-Date(master): Wed Mar 30 15:07:09 UTC 2022 on sn-devel-184
commit 5e1aa469ae61af0442f432e0a2e3bf8c8709616a
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:42:18 2022 -0700
s3: smbd: Preserve the fsp->fsp_name->st bufs across rename_open_files()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 1301e6461393601a4d43cfc465a05114e6ae4662
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:25:54 2022 -0700
s4: torture: Add test_smb2_close_full_information() test to smb2.rename.
Creates a file, opens it again on two different connections
and then renames it. When we close and ask for SMB2_CLOSE_FLAGS_FULL_INFORMATION
we expect this to succeed and return valid data on the handles that did not do
the rename request.
This currently succeeds by accident on master, so we are not
adding a knownfail.d/ file here. When we back-port this test
to 4.16.next, 4.15.next we will add a knownfail.d file.
The rename request zeros out the fsp->fsp_name->st field on the handles
that are open but are not being renamed, marking them as INVALID_STAT.
This should not happen on any open handle. Fix to follow will
preserve the field on rename in both the local connection and
different connection case.
Master gets away with this as in this branch, openat_pathref_fsp(),
which we use in the setup_close_full_information() call to fetch
the SMB2_CLOSE_FLAGS_FULL_INFORMATION data doesn't require an
existing VALID_STAT struct in order to open the file. This
hides the fact the rename zeroed out fsp->fsp_name->st.
4.16.x and 4.15.x don't have this fix, so expose the bug.
Regardless, even in master we should not zero out any
fsp->fsp_name->st values on rename.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 4725ef5c96395dc2f48fab1160a3312d95e21416
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:24:27 2022 -0700
s4: torture: Add CHECK_CREATED macro to smb2/rename.c. Not yet used.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit e862a2d9ec4e7bec1dd58490e9dee47d543b9154
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:23:05 2022 -0700
s4: torture: Add CHECK_VAL macro to smb2/rename.c. Not yet used.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit e01c5992b061d8ed54645fff52a73418013340ab
Author: Jeremy Allison <jra at samba.org>
Date: Mon Mar 28 18:09:20 2022 -0700
s3: tests.py: Only run smb2.rename against fileserver.
No need to run this against nt4_dc or ad_dc.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15038
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/selftest/tests.py | 2 +
source3/smbd/open.c | 20 ++++++
source3/smbd/reply.c | 15 +++++
source4/torture/smb2/rename.c | 147 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 184 insertions(+)
Changeset truncated at 500 lines:
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 2bfb38fdfff..cae09571fe1 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -1017,6 +1017,8 @@ for t in tests:
plansmbtorture4testsuite("smb2.async_dosmode",
"simpleserver",
"//$SERVER_IP/async_dosmode_shadow_copy2 -U$USERNAME%$PASSWORD")
+ elif t == "smb2.rename":
+ plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
elif t == "rpc.wkssvc":
plansmbtorture4testsuite(t, "ad_member", '//$SERVER/tmp -U$DC_USERNAME%$DC_PASSWORD')
elif t == "rpc.srvsvc":
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 9e792a23132..00d8f414c3a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4887,16 +4887,36 @@ void msg_file_was_renamed(struct messaging_context *msg_ctx,
}
if (strcmp(fsp->conn->connectpath, msg->servicepath) == 0) {
+ SMB_STRUCT_STAT fsp_orig_sbuf;
NTSTATUS status;
DBG_DEBUG("renaming file %s from %s -> %s\n",
fsp_fnum_dbg(fsp),
fsp_str_dbg(fsp),
smb_fname_str_dbg(smb_fname));
+
+ /*
+ * The incoming smb_fname here has an
+ * invalid stat struct from synthetic_smb_fname()
+ * above.
+ * Preserve the existing stat from the
+ * open fsp after fsp_set_smb_fname()
+ * overwrites with the invalid stat.
+ *
+ * (We could just copy this into
+ * smb_fname->st, but keep this code
+ * identical to the fix in rename_open_files()
+ * for clarity.
+ *
+ * We will do an fstat before returning
+ * any of this metadata to the client anyway.
+ */
+ fsp_orig_sbuf = fsp->fsp_name->st;
status = fsp_set_smb_fname(fsp, smb_fname);
if (!NT_STATUS_IS_OK(status)) {
DBG_DEBUG("fsp_set_smb_fname failed: %s\n",
nt_errstr(status));
}
+ fsp->fsp_name->st = fsp_orig_sbuf;
} else {
/* TODO. JRA. */
/*
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index e7e39fe23ec..ff3a6504ddf 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6955,6 +6955,7 @@ static void rename_open_files(connection_struct *conn,
for(fsp = file_find_di_first(conn->sconn, id, false); fsp;
fsp = file_find_di_next(fsp, false)) {
+ SMB_STRUCT_STAT fsp_orig_sbuf;
struct file_id_buf idbuf;
/* fsp_name is a relative path under the fsp. To change this for other
sharepaths we need to manipulate relative paths. */
@@ -6973,10 +6974,24 @@ static void rename_open_files(connection_struct *conn,
fsp_str_dbg(fsp),
smb_fname_str_dbg(smb_fname_dst));
+ /*
+ * The incoming smb_fname_dst here has an
+ * invalid stat struct (it must not have
+ * existed for the rename to succeed).
+ * Preserve the existing stat from the
+ * open fsp after fsp_set_smb_fname()
+ * overwrites with the invalid stat.
+ *
+ * We will do an fstat before returning
+ * any of this metadata to the client anyway.
+ */
+ fsp_orig_sbuf = fsp->fsp_name->st;
status = fsp_set_smb_fname(fsp, smb_fname_dst);
if (NT_STATUS_IS_OK(status)) {
did_rename = True;
new_name_hash = fsp->name_hash;
+ /* Restore existing stat. */
+ fsp->fsp_name->st = fsp_orig_sbuf;
}
}
diff --git a/source4/torture/smb2/rename.c b/source4/torture/smb2/rename.c
index 5055cf24344..1b3dca8bcba 100644
--- a/source4/torture/smb2/rename.c
+++ b/source4/torture/smb2/rename.c
@@ -30,6 +30,28 @@
#include "librpc/gen_ndr/security.h"
+#define CHECK_VAL(v, correct) \
+ do { \
+ if ((v) != (correct)) { \
+ torture_result(torture, \
+ TORTURE_FAIL, \
+ "(%s): wrong value for %s got " \
+ "0x%llx - should be 0x%llx\n", \
+ __location__, #v, \
+ (unsigned long long)v, \
+ (unsigned long long)correct); \
+ ret = false; \
+ goto done; \
+ }} while (0)
+
+#define CHECK_CREATED(__io, __created, __attribute) \
+ do { \
+ CHECK_VAL((__io)->out.create_action, NTCREATEX_ACTION_ ## __created); \
+ CHECK_VAL((__io)->out.size, 0); \
+ CHECK_VAL((__io)->out.file_attr, (__attribute)); \
+ CHECK_VAL((__io)->out.reserved2, 0); \
+ } while(0)
+
#define CHECK_STATUS(status, correct) do { \
if (!NT_STATUS_EQUAL(status, correct)) { \
torture_result(torture, TORTURE_FAIL, \
@@ -1415,6 +1437,127 @@ static bool torture_smb2_rename_dir_bench(struct torture_context *tctx,
return true;
}
+static bool test_smb2_close_full_information(struct torture_context *torture,
+ struct smb2_tree *tree1,
+ struct smb2_tree *tree2)
+{
+ union smb_close cl;
+ struct smb2_create io = {0};
+ struct smb2_handle h1 = {{0}};
+ struct smb2_handle h2 = {{0}};
+ struct smb2_handle h3 = {{0}};
+ union smb_setfileinfo sinfo;
+ NTSTATUS status;
+ const char *fname_src = "request.dat";
+ const char *fname_dst = "renamed.dat";
+ bool ret = true;
+
+ /* Start with a tidy share. */
+ smb2_util_unlink(tree1, fname_src);
+ smb2_util_unlink(tree1, fname_dst);
+
+ /* Create the test file, and leave it open. */
+ io.in.fname = fname_src;
+ io.in.desired_access = SEC_FILE_READ_DATA | SEC_FILE_READ_ATTRIBUTE;
+ io.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ status = smb2_create(tree1, tree1, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ h1 = io.out.file.handle;
+ CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+
+ /* Open the test file on the second connection. */
+ ZERO_STRUCT(io);
+ io.in.fname = fname_src;
+ io.in.desired_access = SEC_FILE_READ_DATA | SEC_FILE_READ_ATTRIBUTE;
+ io.in.create_disposition = NTCREATEX_DISP_OPEN;
+ io.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ status = smb2_create(tree2, tree2, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ h2 = io.out.file.handle;
+
+ /* Now open for rename on the first connection. */
+ ZERO_STRUCT(io);
+ io.in.fname = fname_src;
+ io.in.desired_access = SEC_STD_DELETE | SEC_FILE_READ_ATTRIBUTE;
+ io.in.create_disposition = NTCREATEX_DISP_OPEN;
+ io.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ status = smb2_create(tree1, tree1, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ h3 = io.out.file.handle;
+
+ /* Do the rename. */
+ ZERO_STRUCT(sinfo);
+ sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
+ sinfo.rename_information.in.file.handle = h3;
+ sinfo.rename_information.in.new_name = fname_dst;
+ status = smb2_setinfo_file(tree1, &sinfo);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* And close h3. */
+ ZERO_STRUCT(cl.smb2);
+ cl.smb2.level = RAW_CLOSE_SMB2;
+ cl.smb2.in.file.handle = h3;
+ status = smb2_close(tree1, &cl.smb2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ ZERO_STRUCT(h3);
+
+ /*
+ * Close h1 with SMB2_CLOSE_FLAGS_FULL_INFORMATION.
+ * Ensure we get data.
+ */
+ ZERO_STRUCT(cl.smb2);
+ cl.smb2.level = RAW_CLOSE_SMB2;
+ cl.smb2.in.file.handle = h1;
+ cl.smb2.in.flags = SMB2_CLOSE_FLAGS_FULL_INFORMATION;
+ status = smb2_close(tree1, &cl.smb2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ ZERO_STRUCT(h1);
+ CHECK_VAL(cl.smb2.out.file_attr, 0x20);
+
+ /*
+ * Wait 3 seconds for name change to propagate
+ * to the other connection.
+ */
+ sleep(3);
+
+ /*
+ * Close h2 with SMB2_CLOSE_FLAGS_FULL_INFORMATION.
+ * This is on connection2.
+ * Ensure we get data.
+ */
+ ZERO_STRUCT(cl.smb2);
+ cl.smb2.level = RAW_CLOSE_SMB2;
+ cl.smb2.in.file.handle = h2;
+ cl.smb2.in.flags = SMB2_CLOSE_FLAGS_FULL_INFORMATION;
+ status = smb2_close(tree2, &cl.smb2);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ ZERO_STRUCT(h2);
+ CHECK_VAL(cl.smb2.out.file_attr, 0x20);
+
+ done:
+
+ if (h1.data[0] != 0 || h1.data[1] != 0) {
+ smb2_util_close(tree1, h1);
+ }
+ if (h2.data[0] != 0 || h2.data[1] != 0) {
+ smb2_util_close(tree2, h2);
+ }
+ if (h3.data[0] != 0 || h3.data[1] != 0) {
+ smb2_util_close(tree1, h3);
+ }
+
+ smb2_util_unlink(tree1, fname_src);
+ smb2_util_unlink(tree1, fname_dst);
+
+ return ret;
+}
/*
basic testing of SMB2 rename
@@ -1461,6 +1604,10 @@ struct torture_suite *torture_smb2_rename_init(TALLOC_CTX *ctx)
"rename_dir_bench",
torture_smb2_rename_dir_bench);
+ torture_suite_add_2smb2_test(suite,
+ "close-full-information",
+ test_smb2_close_full_information);
+
suite->description = talloc_strdup(suite, "smb2.rename tests");
return suite;
--
Samba Shared Repository
More information about the samba-cvs
mailing list