From f44f3785e24eaa8c11e4f5fe16c737582ca175cc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 1 Mar 2017 18:12:35 +0100 Subject: [PATCH 1/2] s3/smbd: fix deferred open with streams and kernel oplocks ... Bug: https://bugzilla.samba.org/show_bug.cgi?id=7537 --- source3/smbd/open.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++---- source3/smbd/oplock.c | 3 +- source3/smbd/proto.h | 3 +- 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 7bd1086..8d167ae 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -43,6 +43,7 @@ extern const struct generic_mapping file_generic_mapping; struct deferred_open_record { bool delayed_for_oplocks; + bool kernel_oplock; bool async_open; struct file_id id; }; @@ -1544,6 +1545,23 @@ 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; @@ -1935,6 +1953,11 @@ static bool request_timed_out(struct timeval request_time, struct defer_open_state { struct smbXsrv_connection *xconn; uint64_t mid; + struct file_id file_id; + struct timeval request_time; + struct timeval timeout; + bool delayed_for_kernel_oplock; + uint32_t lease_type; }; static void defer_open_done(struct tevent_req *req); @@ -1976,6 +1999,12 @@ 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->delayed_for_kernel_oplock = state->kernel_oplock; + watch_state->lease_type = get_lease_type_from_share_mode( + lck->data); DEBUG(10, ("defering mid %llu\n", (unsigned long long)req->mid)); @@ -2006,6 +2035,10 @@ 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 shedule_req = true; + struct timeval timeout; NTSTATUS status; bool ret; @@ -2019,13 +2052,68 @@ 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->delayed_for_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->delayed_for_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); + shedule_req = false; + } + } + } + + if (shedule_req) { + DBG_DEBUG("scheduling mid %" PRIu64 "\n", state->mid); + + ret = schedule_deferred_open_message_smb(state->xconn, + state->mid); + SMB_ASSERT(ret); + TALLOC_FREE(lck); + TALLOC_FREE(state); + return; } - DEBUG(10, ("scheduling mid %llu\n", (unsigned long long)state->mid)); + 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); - ret = schedule_deferred_open_message_smb(state->xconn, state->mid); + timeout = timeval_sum(&state->request_time, &state->timeout); + ret = tevent_req_set_endtime(watch_req, state->xconn->ev_ctx, timeout); SMB_ASSERT(ret); - TALLOC_FREE(state); + + TALLOC_FREE(lck); } @@ -2142,7 +2230,8 @@ 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) + struct smb_request *req, + bool kernel_oplock) { struct deferred_open_record state; @@ -2172,6 +2261,7 @@ static void schedule_defer_open(struct share_mode_lock *lck, state.delayed_for_oplocks = True; state.async_open = false; state.id = id; + state.kernel_oplock = kernel_oplock; if (!request_timed_out(request_time, timeout)) { defer_open(lck, request_time, timeout, req, &state); @@ -2822,7 +2912,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, state.id = fsp->file_id; defer_open(NULL, request_time, timeval_set(0, 0), req, &state); - DEBUG(10, ("No share mode lock found after " + DEBUG(1, ("No share mode lock found after " "EWOULDBLOCK, retrying sync\n")); return NT_STATUS_SHARING_VIOLATION; } @@ -2834,7 +2924,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if (delay_for_oplock(fsp, 0, lease, lck, false, create_disposition, first_open_attempt)) { schedule_defer_open(lck, fsp->file_id, request_time, - req); + req, true); TALLOC_FREE(lck); DEBUG(10, ("Sent oplock break request to kernel " "oplock holder\n")); @@ -2958,7 +3048,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, fsp, oplock_request, lease, lck, NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION), create_disposition, first_open_attempt)) { - schedule_defer_open(lck, fsp->file_id, request_time, req); + schedule_defer_open(lck, fsp->file_id, request_time, req, true); TALLOC_FREE(lck); fd_close(fsp); return NT_STATUS_SHARING_VIOLATION; diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c index 8e29f0d..e8a5a16 100644 --- a/source3/smbd/oplock.c +++ b/source3/smbd/oplock.c @@ -172,7 +172,8 @@ uint32_t map_oplock_to_lease_type(uint16_t op_type) return ret; } -uint32_t get_lease_type(struct share_mode_data *d, struct share_mode_entry *e) +uint32_t get_lease_type(const struct share_mode_data *d, + const struct share_mode_entry *e) { if (e->op_type == LEASE_OPLOCK) { return d->leases[e->lease_idx].current_state; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 7ccb12a..06ab56f 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -704,7 +704,8 @@ NTSTATUS get_relative_fid_filename(connection_struct *conn, /* The following definitions come from smbd/oplock.c */ uint32_t map_oplock_to_lease_type(uint16_t op_type); -uint32_t get_lease_type(struct share_mode_data *d, struct share_mode_entry *e); +uint32_t get_lease_type(const struct share_mode_data *d, + const struct share_mode_entry *e); bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck); void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp); -- 2.9.3 From b2705d73be85125593d50156ec4cda6521c0b280 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 1 Mar 2017 18:13:35 +0100 Subject: [PATCH 2/2] s4/torture: some tests for kernel oplocks Bug: https://bugzilla.samba.org/show_bug.cgi?id=7537 --- selftest/target/Samba3.pm | 5 ++ source3/selftest/tests.py | 2 + source4/selftest/tests.py | 2 +- source4/torture/smb2/oplock.c | 140 ++++++++++++++++++++++++++++++++++++++++++ source4/torture/smb2/smb2.c | 1 + 5 files changed, 149 insertions(+), 1 deletion(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 775dc16..d754b5f 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1860,6 +1860,11 @@ sub provision($$$$$$$$) [mangle_illegal] copy = tmp mangled names = illegal + +[kernel_oplocks] + copy = tmp + kernel oplocks = yes + vfs objects = streams_xattr xattr_tdb "; close(CONF); diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index f180710..762c4ad 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -436,6 +436,8 @@ for t in tests: plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --signing=required') elif t == "smb2.dosmode": plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/dosmode -U$USERNAME%$PASSWORD') + elif t == "smb2.kernel_oplocks": + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER/kernel_oplocks -U$USERNAME%$PASSWORD') elif t == "vfs.acl_xattr": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') else: diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index ac601f5..f55ac43 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -303,7 +303,7 @@ for t in nbt_tests: ntvfsargs = ["--option=torture:sharedelay=100000", "--option=torture:oplocktimeout=3", "--option=torture:writetimeupdatedelay=500000"] # Filter smb2 tests that should not run against ad_dc_ntvfs -smb2_s3only = ["smb2.change_notify_disabled", "smb2.dosmode", "smb2.credits"] +smb2_s3only = ["smb2.change_notify_disabled", "smb2.dosmode", "smb2.credits", "smb2.kernel_oplocks"] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] #The QFILEINFO-IPC test needs to be on ipc$ diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c index ead341d..8609a56 100644 --- a/source4/torture/smb2/oplock.c +++ b/source4/torture/smb2/oplock.c @@ -4204,3 +4204,143 @@ bool test_smb2_hold_oplock(struct torture_context *tctx, talloc_free(mem_ctx); return true; } + + +static bool test_smb2_kernel_oplocks1(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "test_kernel_oplock1.dat"; + NTSTATUS status; + bool ret = true; + struct smb2_create create; + struct smb2_handle h1 = {{0}}, h2 = {{0}}; + + smb2_util_unlink(tree, fname); + + tree->session->transport->oplock.handler = torture_oplock_handler; + tree->session->transport->oplock.private_data = tree; + ZERO_STRUCT(break_info); + + 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_NONE; + create.in.create_disposition = NTCREATEX_DISP_OPEN_IF; + 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"); + h1 = create.out.file.handle; + + torture_assert_goto(tctx, create.out.oplock_level == SMB2_OPLOCK_LEVEL_EXCLUSIVE, ret, done, + "Oplock level is not SMB2_OPLOCK_LEVEL_EXCLUSIVE\n"); + + 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_IF; + create.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; + create.in.fname = fname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto(tctx, status, NT_STATUS_SHARING_VIOLATION, ret, done, + "Open didn't return NT_STATUS_SHARING_VIOLATION\n"); + h2 = create.out.file.handle; + + torture_wait_for_oplock_break(tctx); + if (break_info.count != 0) { + torture_warning(tctx, "Open caused oplock break\n"); + } + + smb2_util_close(tree, h1); + smb2_util_close(tree, h2); + +done: + if (!smb2_util_handle_empty(h1)) { + smb2_util_close(tree, h1); + } + if (!smb2_util_handle_empty(h2)) { + smb2_util_close(tree, h2); + } + smb2_util_unlink(tree, fname); + return ret; +} + +static bool test_smb2_kernel_oplocks2(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "test_kernel_oplock2.dat"; + const char *sname = "test_kernel_oplock2.dat:foo"; + NTSTATUS status; + bool ret = true; + struct smb2_create create; + struct smb2_handle h1 = {{0}}, h2 = {{0}}; + + smb2_util_unlink(tree, fname); + + tree->session->transport->oplock.handler = torture_oplock_handler; + tree->session->transport->oplock.private_data = tree; + ZERO_STRUCT(break_info); + + 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_NONE; + create.in.create_disposition = NTCREATEX_DISP_OPEN_IF; + 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"); + h1 = create.out.file.handle; + + torture_assert_goto(tctx, create.out.oplock_level == SMB2_OPLOCK_LEVEL_EXCLUSIVE, ret, done, + "Oplock level is not SMB2_OPLOCK_LEVEL_EXCLUSIVE\n"); + + 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_IF; + create.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; + create.in.fname = sname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "Error opening the file\n"); + h2 = create.out.file.handle; + + torture_wait_for_oplock_break(tctx); + if (break_info.count != 0) { + torture_warning(tctx, "Stream open caused oplock break\n"); + } + + smb2_util_close(tree, h1); + smb2_util_close(tree, h2); + +done: + if (!smb2_util_handle_empty(h1)) { + smb2_util_close(tree, h1); + } + if (!smb2_util_handle_empty(h2)) { + smb2_util_close(tree, h2); + } + smb2_util_unlink(tree, fname); + return ret; +} + +struct torture_suite *torture_smb2_kernel_oplocks_init(void) +{ + struct torture_suite *suite = + torture_suite_create(talloc_autofree_context(), "kernel_oplocks"); + + torture_suite_add_1smb2_test(suite, "kernel_oplocks1", test_smb2_kernel_oplocks1); + torture_suite_add_1smb2_test(suite, "kernel_oplocks2", test_smb2_kernel_oplocks2); + + suite->description = talloc_strdup(suite, "SMB2-KERNEL-OPLOCK tests"); + + return suite; +} diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index 9f0f6b3..9f8cbe7 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -163,6 +163,7 @@ NTSTATUS torture_smb2_init(void) torture_suite_add_suite(suite, torture_smb2_lease_init()); torture_suite_add_suite(suite, torture_smb2_compound_init()); torture_suite_add_suite(suite, torture_smb2_oplocks_init()); + torture_suite_add_suite(suite, torture_smb2_kernel_oplocks_init()); torture_suite_add_suite(suite, torture_smb2_streams_init()); torture_suite_add_suite(suite, torture_smb2_ioctl_init()); torture_suite_add_suite(suite, torture_smb2_rename_init()); -- 2.9.3