The Samba-Bugzilla – Attachment 13772 Details for
Bug 13058
Kernel oplock can remain not broken if oplock holder closes the file and another process catches the lease.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.7.next.
bug-13058-4.7.patch (text/plain), 13.50 KB, created by
Jeremy Allison
on 2017-11-11 17:09:14 UTC
(
hide
)
Description:
git-am fix for 4.7.next.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2017-11-11 17:09:14 UTC
Size:
13.50 KB
patch
obsolete
>From 3149c619d271145885b35220e79862142b1e073b Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 3 Nov 2017 21:47:01 +0000 >Subject: [PATCH 1/2] Revert "s3/smbd: fix deferred open with streams and > kernel oplocks" >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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 <jra@samba.org> >Reviewed-by: Ralph Böhme <slow@samba.org> >(cherry picked from commit 62a556d5c8ce0650e3a2095ee62bea16c8eab1d5) >--- > source3/smbd/open.c | 115 +++++----------------------------------------------- > 1 file changed, 11 insertions(+), 104 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 7781a6f86a7..89a267b0634 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); > } > } > >-- >2.14.1 > > >From d1f69906f440861afe7b9c1f47909bc9394a519a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 3 Nov 2017 12:02:17 -0700 >Subject: [PATCH 2/2] s4: torture: kernel_oplocks. Create a regression test > case for bug #13058. >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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 <jra@samba.org> >Reviewed-by: Ralph Böhme <slow@samba.org> >(cherry picked from commit 15597a95ecd2d1c2b7edce4942d489c95796951f) >--- > source4/torture/smb2/oplock.c | 117 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 117 insertions(+) > >diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c >index e0db5ecb50d..3290ed42d8c 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"); > >-- >2.14.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 13058
:
13639
|
13742
|
13764
|
13770
|
13771
| 13772 |
13773