From 6fb7dabcc306452588535d090e2e80e0a161e26e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 27 Oct 2017 12:48:14 -0700 Subject: [PATCH 1/3] WIP. Always use O_NONBLOCK for kernel oplocks. Signed-off-by: Jeremy Allison --- source3/smbd/open.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 7781a6f86a7..8e3227aa3b4 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3253,20 +3253,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, flags2 &= ~(O_CREAT|O_TRUNC); } - if (first_open_attempt && lp_kernel_oplocks(SNUM(conn))) { + if (lp_kernel_oplocks(SNUM(conn))) { /* * With kernel oplocks the open breaking an oplock * blocks until the oplock holder has given up the - * oplock or closed the file. We prevent this by first + * oplock or closed the file. We prevent this by always * trying to open the file with O_NONBLOCK (see "man - * fcntl" on Linux). For the second try, triggered by - * an oplock break response, we do not need this - * anymore. - * - * This is true under the assumption that only Samba - * requests kernel oplocks. Once someone else like - * NFSv4 starts to use that API, we will have to - * modify this by communicating with the NFSv4 server. + * fcntl" on Linux). */ flags2 |= O_NONBLOCK; } -- 2.15.0.rc2.357.g7e34df9404-goog From fb6f05720ce8be339feab1b7801ff983eb858984 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Oct 2017 15:05:00 -0700 Subject: [PATCH 2/3] WIP: Revert "s3/smbd: fix deferred open with streams and kernel oplocks" This reverts commit b35a296a27a0807c780f2a9e7af2f2e93feefaa8. --- 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 8e3227aa3b4..8dc41fd7180 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); } /**************************************************************************** @@ -3353,7 +3260,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")); @@ -3486,7 +3393,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; @@ -3590,7 +3497,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.15.0.rc2.357.g7e34df9404-goog From 67f7d1852fabee994212e4c6fd171228a39167a2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Oct 2017 15:22:17 -0700 Subject: [PATCH 3/3] WIP: Real fix for problem b35a296a27a0807c780f2a9e7af2f2e93feefaa8 tried to fix. If we're opening a stream file and have fsp->base_fsp open, ensure we close this handle under the share mode lock *before* setting a watchpoint on the share mode record. Otherwise the cleanup close of base_fsp in create_file_unixpath() triggers the record to fire when we don't have any real change to examine. Signed-off-by: Jeremy Allison --- source3/smbd/open.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 8dc41fd7180..f0916def54c 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3259,6 +3259,17 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, create_disposition, first_open_attempt); if (delay) { + if (fsp->base_fsp) { + /* + * We're opening a stream here. + * Ensure we remove any base_fsp + * record under the lock *before* + * setting the watch. + */ + close_file(req, fsp->base_fsp, ERROR_CLOSE); + fsp->base_fsp = NULL; + } + schedule_defer_open(lck, fsp->file_id, request_time, req); TALLOC_FREE(lck); @@ -3392,6 +3403,16 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, create_disposition, first_open_attempt); if (delay) { + if (fsp->base_fsp) { + /* + * We're opening a stream here. + * Ensure we remove any base_fsp + * record under the lock *before* + * setting the watch. + */ + close_file(req, fsp->base_fsp, ERROR_CLOSE); + fsp->base_fsp = NULL; + } schedule_defer_open(lck, fsp->file_id, request_time, req); TALLOC_FREE(lck); @@ -5176,6 +5197,13 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, fsp); if(!NT_STATUS_IS_OK(status)) { + if (base_fsp != NULL && fsp->base_fsp == NULL) { + /* + * open_file_ntcreate() already closed + * base_fsp as part of erroring out. + */ + base_fsp = NULL; + } file_free(req, fsp); fsp = NULL; } -- 2.15.0.rc2.357.g7e34df9404-goog