The branch, master has been updated
via 20cfcb7 tevent: Fix a race condition
via 71720e2 libcli: Fix a typo
via 15597a9 s4: torture: kernel_oplocks. Create a regression test case
for bug #13058.
via 62a556d Revert "s3/smbd: fix deferred open with streams and kernel
oplocks"
from 732ba3c vfs_glusterfs: include glusterfs/api/glfs.h without relying
on -I options
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 20cfcb7dbc5dd099384b76a76e3d35cf627100b6
Author: Volker Lendecke <[email protected]>
Date: Fri Nov 10 21:22:26 2017 +0100
tevent: Fix a race condition
We can't rely on tctx to exist after we unlocked the mutex. It took a
while, but this does lead to data corruption. If *tctx is replaced with
something where tctx->wakeup_fd points to a real, existing file
descriptor, we're screwed. And by screwed, this means file corruption
on disk.
Again. I am not tall enough for this business.
http://bholley.net/blog/2015/must-be-this-tall-to-write-multi-threaded-code.html
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13130
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
Autobuild-User(master): Jeremy Allison <[email protected]>
Autobuild-Date(master): Sat Nov 11 03:20:09 CET 2017 on sn-devel-144
commit 71720e2e9c414f8ae823a5c6cfa624a4d8683926
Author: Volker Lendecke <[email protected]>
Date: Mon Oct 30 13:46:04 2017 +0100
libcli: Fix a typo
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Richard Sharpe <[email protected]>
commit 15597a95ecd2d1c2b7edce4942d489c95796951f
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 3 12:02:17 2017 -0700
s4: torture: kernel_oplocks. Create a regression test case for bug #13058.
It implements the following test case:
1. client of smbd-1 opens the file and sets the oplock.
2. client of smbd-2 tries to open the file. open() fails(EAGAIN) and open
is deferred.
3. client of smbd-1 sends oplock break request to the client.
4. client of smbd-1 closes the file.
5. client of smbd-1 opens the file and sets the oplock.
6. client of smbd-2 calls defer_open_done(), sees that the file lease was
not changed
and does not reschedule open.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13058
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Ralph Böhme <[email protected]>
commit 62a556d5c8ce0650e3a2095ee62bea16c8eab1d5
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 3 21:47:01 2017 +0000
Revert "s3/smbd: fix deferred open with streams and kernel oplocks"
This reverts commit b35a296a27a0807c780f2a9e7af2f2e93feefaa8.
This was the cause of
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13058
1. client of smbd-1 opens the file and sets the oplock.
2. client of smbd-2 tries to open the file. open() fails(EAGAIN) and open
is deferred.
3. client of smbd-1 sends oplock break request to the client.
4. client of smbd-1 closes the file.
5. client of smbd-1 opens the file and sets the oplock.
6. client of smbd-2 calls defer_open_done(), sees that the file lease was
not changed
and does not reschedule open.
and is no longer needed now vfs_streams_xattr.c no longer opens
the base file internally.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Ralph Böhme <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
lib/tevent/tevent_threads.c | 6 ++-
libcli/smb/smbXcli_base.c | 2 +-
source3/smbd/open.c | 115 ++++-------------------------------------
source4/torture/smb2/oplock.c | 117 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+), 107 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/tevent/tevent_threads.c b/lib/tevent/tevent_threads.c
index 4d1a880..2e83f1b 100644
--- a/lib/tevent/tevent_threads.c
+++ b/lib/tevent/tevent_threads.c
@@ -451,7 +451,7 @@ void _tevent_threaded_schedule_immediate(struct
tevent_threaded_context *tctx,
{
#ifdef HAVE_PTHREAD
struct tevent_context *ev;
- int ret;
+ int ret, wakeup_fd;
ret = pthread_mutex_lock(&tctx->event_ctx_mutex);
if (ret != 0) {
@@ -495,6 +495,8 @@ void _tevent_threaded_schedule_immediate(struct
tevent_threaded_context *tctx,
abort();
}
+ wakeup_fd = tctx->wakeup_fd;
+
ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
if (ret != 0) {
abort();
@@ -510,7 +512,7 @@ void _tevent_threaded_schedule_immediate(struct
tevent_threaded_context *tctx,
* than a noncontended one. So I'd opt for the lower footprint
* initially. Maybe we have to change that later.
*/
- tevent_common_wakeup_fd(tctx->wakeup_fd);
+ tevent_common_wakeup_fd(wakeup_fd);
#else
/*
* tevent_threaded_context_create() returned NULL with ENOSYS...
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 7322380..6335ce0 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -878,7 +878,7 @@ static NTSTATUS smbXcli_req_cancel_write_req(struct
tevent_req *req)
/*
* Check if it's possible to cancel the request.
- * If the result is true it's not to late.
+ * If the result is true it's not too late.
* See writev_cancel().
*/
ok = tevent_req_cancel(state->write_req);
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 7781a6f..89a267b 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1896,23 +1896,6 @@ static bool delay_for_oplock(files_struct *fsp,
return delay;
}
-/**
- * Return lease or oplock state from a share mode
- **/
-static uint32_t get_lease_type_from_share_mode(const struct share_mode_data *d)
-{
- uint32_t e_lease_type = 0;
- uint32_t i;
-
- for (i=0; i < d->num_share_modes; i++) {
- struct share_mode_entry *e = &d->share_modes[i];
-
- e_lease_type |= get_lease_type(d, e);
- }
-
- return e_lease_type;
-}
-
static bool file_has_brlocks(files_struct *fsp)
{
struct byte_range_lock *br_lck;
@@ -2325,11 +2308,6 @@ static struct deferred_open_record
*deferred_open_record_create(
struct defer_open_state {
struct smbXsrv_connection *xconn;
uint64_t mid;
- struct file_id file_id;
- struct timeval request_time;
- struct timeval timeout;
- bool kernel_oplock;
- uint32_t lease_type;
};
static void defer_open_done(struct tevent_req *req);
@@ -2348,7 +2326,6 @@ static void defer_open(struct share_mode_lock *lck,
struct timeval timeout,
struct smb_request *req,
bool delayed_for_oplocks,
- bool kernel_oplock,
struct file_id id)
{
struct deferred_open_record *open_rec = NULL;
@@ -2360,12 +2337,11 @@ static void defer_open(struct share_mode_lock *lck,
abs_timeout = timeval_sum(&request_time, &timeout);
DBG_DEBUG("request time [%s] timeout [%s] mid [%" PRIu64 "] "
- "delayed_for_oplocks [%s] kernel_oplock [%s] file_id [%s]\n",
+ "delayed_for_oplocks [%s] file_id [%s]\n",
timeval_string(talloc_tos(), &request_time, false),
timeval_string(talloc_tos(), &abs_timeout, false),
req->mid,
delayed_for_oplocks ? "yes" : "no",
- kernel_oplock ? "yes" : "no",
file_id_string_tos(&id));
open_rec = deferred_open_record_create(delayed_for_oplocks,
@@ -2382,11 +2358,6 @@ static void defer_open(struct share_mode_lock *lck,
}
watch_state->xconn = req->xconn;
watch_state->mid = req->mid;
- watch_state->file_id = lck->data->id;
- watch_state->request_time = request_time;
- watch_state->timeout = timeout;
- watch_state->kernel_oplock = kernel_oplock;
- watch_state->lease_type = get_lease_type_from_share_mode(lck->data);
DBG_DEBUG("defering mid %" PRIu64 "\n", req->mid);
@@ -2416,12 +2387,8 @@ static void defer_open_done(struct tevent_req *req)
{
struct defer_open_state *state = tevent_req_callback_data(
req, struct defer_open_state);
- struct tevent_req *watch_req = NULL;
- struct share_mode_lock *lck = NULL;
- bool schedule_req = true;
- struct timeval timeout;
NTSTATUS status;
- bool ok;
+ bool ret;
status = dbwrap_watched_watch_recv(req, talloc_tos(), NULL, NULL,
NULL);
@@ -2433,72 +2400,13 @@ static void defer_open_done(struct tevent_req *req)
* Even if it failed, retry anyway. TODO: We need a way to
* tell a re-scheduled open about that error.
*/
- if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT) &&
- state->kernel_oplock)
- {
- /*
- * If we reschedule but the kernel oplock is still hold
- * we would block in the second open as that will be a
- * blocking open attempt.
- */
- exit_server("Kernel oplock holder didn't "
- "respond to break message");
- }
- }
-
- if (state->kernel_oplock) {
- lck = get_existing_share_mode_lock(talloc_tos(),
state->file_id);
- if (lck != NULL) {
- uint32_t lease_type;
-
- lease_type = get_lease_type_from_share_mode(lck->data);
-
- if ((lease_type != 0) &&
- (lease_type == state->lease_type))
- {
- DBG_DEBUG("Unchanged lease: %" PRIu32 "\n",
- lease_type);
- schedule_req = false;
- }
- }
- }
-
- if (schedule_req) {
- DBG_DEBUG("scheduling mid %" PRIu64 "\n", state->mid);
-
- ok = schedule_deferred_open_message_smb(state->xconn,
- state->mid);
- if (!ok) {
- exit_server("schedule_deferred_open_message_smb
failed");
- }
- TALLOC_FREE(lck);
- TALLOC_FREE(state);
- return;
- }
-
- DBG_DEBUG("Keep waiting for oplock release for [%s/%s%s] "
- "mid: %" PRIu64 "\n",
- lck->data->servicepath,
- lck->data->base_name,
- lck->data->stream_name ? lck->data->stream_name : "",
- state->mid);
-
- watch_req = dbwrap_watched_watch_send(state,
- state->xconn->ev_ctx,
- lck->data->record,
- (struct server_id){0});
- if (watch_req == NULL) {
- exit_server("Could not watch share mode record");
}
- tevent_req_set_callback(watch_req, defer_open_done, state);
- timeout = timeval_sum(&state->request_time, &state->timeout);
- ok = tevent_req_set_endtime(watch_req, state->xconn->ev_ctx, timeout);
- if (!ok) {
- exit_server("tevent_req_set_endtime failed");
- }
+ DEBUG(10, ("scheduling mid %llu\n", (unsigned long long)state->mid));
- TALLOC_FREE(lck);
+ ret = schedule_deferred_open_message_smb(state->xconn, state->mid);
+ SMB_ASSERT(ret);
+ TALLOC_FREE(state);
}
/**
@@ -2649,8 +2557,7 @@ static NTSTATUS fcb_or_dos_open(struct smb_request *req,
static void schedule_defer_open(struct share_mode_lock *lck,
struct file_id id,
struct timeval request_time,
- struct smb_request *req,
- bool kernel_oplock)
+ struct smb_request *req)
{
/* This is a relative time, added to the absolute
request_time value to get the absolute timeout time.
@@ -2674,7 +2581,7 @@ static void schedule_defer_open(struct share_mode_lock
*lck,
return;
}
- defer_open(lck, request_time, timeout, req, true, kernel_oplock, id);
+ defer_open(lck, request_time, timeout, req, true, id);
}
/****************************************************************************
@@ -3360,7 +3267,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
first_open_attempt);
if (delay) {
schedule_defer_open(lck, fsp->file_id, request_time,
- req, true);
+ req);
TALLOC_FREE(lck);
DEBUG(10, ("Sent oplock break request to kernel "
"oplock holder\n"));
@@ -3493,7 +3400,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
first_open_attempt);
if (delay) {
schedule_defer_open(lck, fsp->file_id,
- request_time, req, false);
+ request_time, req);
TALLOC_FREE(lck);
fd_close(fsp);
return NT_STATUS_SHARING_VIOLATION;
@@ -3597,7 +3504,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
if (!request_timed_out(request_time, timeout)) {
defer_open(lck, request_time, timeout, req,
- false, false, id);
+ false, id);
}
}
diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c
index e0db5ec..3290ed4 100644
--- a/source4/torture/smb2/oplock.c
+++ b/source4/torture/smb2/oplock.c
@@ -4674,6 +4674,122 @@ done:
return ret;
}
+/**
+ * Recreate regression test from bug:
+ *
+ * https://bugzilla.samba.org/show_bug.cgi?id=13058
+ *
+ * 1. smbd-1 opens the file and sets the oplock
+ * 2. smbd-2 tries to open the file. open() fails(EAGAIN) and open is deferred.
+ * 3. smbd-1 sends oplock break request to the client.
+ * 4. smbd-1 closes the file.
+ * 5. smbd-1 opens the file and sets the oplock.
+ * 6. smbd-2 calls defer_open_done(), and should re-break the oplock.
+ **/
+
+static bool test_smb2_kernel_oplocks7(struct torture_context *tctx,
+ struct smb2_tree *tree,
+ struct smb2_tree *tree2)
+{
+ const char *fname = "test_kernel_oplock7.dat";
+ NTSTATUS status;
+ bool ret = true;
+ struct smb2_create create;
+ struct smb2_handle h1 = {{0}}, h2 = {{0}};
+ struct smb2_create create_2;
+ struct smb2_create io;
+ struct smb2_request *req;
+
+ smb2_util_unlink(tree, fname);
+ status = torture_smb2_testfile(tree, fname, &h1);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Error creating testfile\n");
+ smb2_util_close(tree, h1);
+ ZERO_STRUCT(h1);
+
+ /* Close the open file on break. */
+ tree->session->transport->oplock.handler = torture_oplock_handler_close;
+ tree->session->transport->oplock.private_data = tree;
+ ZERO_STRUCT(break_info);
+
+ /* 1 - open file with oplock */
+ ZERO_STRUCT(create);
+ create.in.desired_access = SEC_RIGHTS_FILE_ALL;
+ create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
+ create.in.create_disposition = NTCREATEX_DISP_OPEN;
+ create.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ create.in.fname = fname;
+ create.in.oplock_level = SMB2_OPLOCK_LEVEL_EXCLUSIVE;
+
+ status = smb2_create(tree, tctx, &create);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Error opening the file\n");
+ CHECK_VAL(create.out.oplock_level, SMB2_OPLOCK_LEVEL_EXCLUSIVE);
+
+ /* 2 - open file to break oplock */
+ ZERO_STRUCT(create_2);
+ create_2.in.desired_access = SEC_RIGHTS_FILE_ALL;
+ create_2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ create_2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
+ create_2.in.create_disposition = NTCREATEX_DISP_OPEN;
+ create_2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ create_2.in.fname = fname;
+ create_2.in.oplock_level = SMB2_OPLOCK_LEVEL_NONE;
+
+ /* Open on tree2 - should cause a break on tree */
+ req = smb2_create_send(tree2, &create_2);
+ torture_assert(tctx, req != NULL, "smb2_create_send");
+
+ /* The oplock break handler should close the file. */
+ /* Steps 3 & 4. */
+ torture_wait_for_oplock_break(tctx);
+
+ tree->session->transport->oplock.handler = torture_oplock_handler;
+
+ /*
+ * 5 - re-open on tree. NB. There is a race here
+ * depending on which smbd goes first. We either get
+ * an oplock level of SMB2_OPLOCK_LEVEL_EXCLUSIVE if
+ * the close and re-open on tree is processed first, or
+ * SMB2_OPLOCK_LEVEL_NONE if the pending create on
+ * tree2 is processed first.
+ */
+ status = smb2_create(tree, tctx, &create);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Error opening the file\n");
+
+ h1 = create.out.file.handle;
+ if (create.out.oplock_level != SMB2_OPLOCK_LEVEL_EXCLUSIVE &&
+ create.out.oplock_level != SMB2_OPLOCK_LEVEL_NONE) {
+ torture_result(tctx,
+ TORTURE_FAIL,
+ "(%s): wrong value for oplock got 0x%x\n",
+ __location__,
+ (unsigned int)create.out.oplock_level);
+ ret = false;
+ goto done;
+
+ }
+
+ /* 6 - retrieve the second open. */
+ status = smb2_create_recv(req, tctx, &io);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Error opening the file\n");
+ h2 = io.out.file.handle;
+ CHECK_VAL(io.out.oplock_level, SMB2_OPLOCK_LEVEL_NONE);
+
+done:
+ if (!smb2_util_handle_empty(h1)) {
+ smb2_util_close(tree, h1);
+ }
+ if (!smb2_util_handle_empty(h2)) {
+ smb2_util_close(tree2, h2);
+ }
+ smb2_util_unlink(tree, fname);
+ return ret;
+}
+
struct torture_suite *torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx)
{
struct torture_suite *suite =
@@ -4685,6 +4801,7 @@ struct torture_suite
*torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx)
torture_suite_add_1smb2_test(suite, "kernel_oplocks4",
test_smb2_kernel_oplocks4);
torture_suite_add_1smb2_test(suite, "kernel_oplocks5",
test_smb2_kernel_oplocks5);
torture_suite_add_2smb2_test(suite, "kernel_oplocks6",
test_smb2_kernel_oplocks6);
+ torture_suite_add_2smb2_test(suite, "kernel_oplocks7",
test_smb2_kernel_oplocks7);
suite->description = talloc_strdup(suite, "SMB2-KERNEL-OPLOCK tests");
--
Samba Shared Repository
