[PATCH] Fix ancient race condition in our mkdir code.
Jeremy Allison
jra at samba.org
Wed Sep 23 01:16:03 UTC 2015
Found by Max @ LoadDynamix.
Contains regression test that always
hits the race :-).
Please review !
Cheers,
Jeremy.
-------------- next part --------------
>From e4ae6db7c160f8766f68bcc19eb78173dc5f5f9f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 22 Sep 2015 18:02:53 -0700
Subject: [PATCH 1/2] s3: smbd: Fix mkdir race condition.
Found by Max of LoadDynamix <adx.forum at gmail.com>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11486
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/open.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 809fa35..162e834 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3488,6 +3488,25 @@ static NTSTATUS open_directory(connection_struct *conn,
nt_errstr(status)));
return status;
}
+
+ /*
+ * If mkdir_internal() returned
+ * NT_STATUS_OBJECT_NAME_COLLISION
+ * we still must lstat the path.
+ */
+
+ if (SMB_VFS_LSTAT(conn, smb_dname)
+ == -1) {
+ DEBUG(2, ("Could not stat "
+ "directory '%s' just "
+ "opened: %s\n",
+ smb_fname_str_dbg(
+ smb_dname),
+ strerror(errno)));
+ return map_nt_error_from_unix(
+ errno);
+ }
+
info = FILE_WAS_OPENED;
}
}
--
2.1.4
>From 7cd3e781298efc5613747765edc205c6c3a3964c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 22 Sep 2015 18:01:22 -0700
Subject: [PATCH 2/2] s4: torture: Test mkdir race condition.
Found by Max of LoadDynamix <adx.forum at gmail.com>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11486
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/torture/smb2/create.c | 151 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 151 insertions(+)
diff --git a/source4/torture/smb2/create.c b/source4/torture/smb2/create.c
index 44650b5..68dbbc1 100644
--- a/source4/torture/smb2/create.c
+++ b/source4/torture/smb2/create.c
@@ -1518,6 +1518,156 @@ done:
}
/*
+ test SMB2 mkdir with OPEN_IF on the same name twice.
+ Must use 2 connections to hit the race.
+*/
+
+static bool test_mkdir_dup(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = "mkdir_dup";
+ NTSTATUS status;
+ bool ret = true;
+ union smb_open io;
+ struct smb2_tree **trees;
+ struct smb2_request **requests;
+ union smb_open *ios;
+ int i, num_files = 2;
+ int num_ok = 0;
+ int num_created = 0;
+ int num_existed = 0;
+
+ torture_comment(tctx,
+ "Testing SMB2 Create Directory with multiple connections\n");
+ trees = talloc_array(tctx, struct smb2_tree *, num_files);
+ requests = talloc_array(tctx, struct smb2_request *, num_files);
+ ios = talloc_array(tctx, union smb_open, num_files);
+ if ((tctx->ev == NULL) || (trees == NULL) || (requests == NULL) ||
+ (ios == NULL)) {
+ torture_fail(tctx, ("talloc failed\n"));
+ ret = false;
+ goto done;
+ }
+
+ tree->session->transport->options.request_timeout = 60;
+
+ for (i=0; i<num_files; i++) {
+ if (!torture_smb2_connection(tctx, &(trees[i]))) {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "Could not open %d'th connection\n", i));
+ ret = false;
+ goto done;
+ }
+ trees[i]->session->transport->options.request_timeout = 60;
+ }
+
+ /* cleanup */
+ smb2_util_unlink(tree, fname);
+ smb2_util_rmdir(tree, fname);
+
+ /*
+ base ntcreatex parms
+ */
+ ZERO_STRUCT(io.smb2);
+ io.generic.level = RAW_OPEN_SMB2;
+ io.smb2.in.desired_access = SEC_RIGHTS_FILE_ALL;
+ io.smb2.in.alloc_size = 0;
+ io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+ NTCREATEX_SHARE_ACCESS_WRITE|
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
+ io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+ io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io.smb2.in.security_flags = 0;
+ io.smb2.in.fname = fname;
+ io.smb2.in.create_flags = 0;
+
+ for (i=0; i<num_files; i++) {
+ ios[i] = io;
+ requests[i] = smb2_create_send(trees[i], &(ios[i].smb2));
+ if (requests[i] == NULL) {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "could not send %d'th request\n", i));
+ ret = false;
+ goto done;
+ }
+ }
+
+ torture_comment(tctx, "waiting for replies\n");
+ while (1) {
+ bool unreplied = false;
+ for (i=0; i<num_files; i++) {
+ if (requests[i] == NULL) {
+ continue;
+ }
+ if (requests[i]->state < SMB2_REQUEST_DONE) {
+ unreplied = true;
+ break;
+ }
+ status = smb2_create_recv(requests[i], tctx,
+ &(ios[i].smb2));
+
+ if (NT_STATUS_IS_OK(status)) {
+ num_ok += 1;
+
+ if (ios[i].smb2.out.create_action ==
+ NTCREATEX_ACTION_CREATED) {
+ num_created++;
+ }
+ if (ios[i].smb2.out.create_action ==
+ NTCREATEX_ACTION_EXISTED) {
+ num_existed++;
+ }
+ } else {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "File %d returned status %s\n", i,
+ nt_errstr(status)));
+ }
+
+
+ requests[i] = NULL;
+ }
+ if (!unreplied) {
+ break;
+ }
+
+ if (tevent_loop_once(tctx->ev) != 0) {
+ torture_fail(tctx, "tevent_loop_once failed\n");
+ ret = false;
+ goto done;
+ }
+ }
+
+ if (num_ok != 2) {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "num_ok == %d\n", num_ok));
+ ret = false;
+ }
+ if (num_created != 1) {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "num_created == %d\n", num_created));
+ ret = false;
+ }
+ if (num_existed != 1) {
+ torture_fail(tctx,
+ talloc_asprintf(tctx,
+ "num_existed == %d\n", num_existed));
+ ret = false;
+ }
+done:
+ smb2_deltree(tree, fname);
+
+ return ret;
+}
+
+
+/*
basic testing of SMB2 read
*/
struct torture_suite *torture_smb2_create_init(void)
@@ -1535,6 +1685,7 @@ struct torture_suite *torture_smb2_create_init(void)
torture_suite_add_1smb2_test(suite, "aclfile", test_create_acl_file);
torture_suite_add_1smb2_test(suite, "acldir", test_create_acl_dir);
torture_suite_add_1smb2_test(suite, "nulldacl", test_create_null_dacl);
+ torture_suite_add_1smb2_test(suite, "mkdir-dup", test_mkdir_dup);
suite->description = talloc_strdup(suite, "SMB2-CREATE tests");
--
2.1.4
More information about the samba-technical
mailing list