From d16fbf3d8ab05fb5a0336bf5fc7c911fbbe89713 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 27 May 2011 12:22:04 -0700 Subject: [PATCH] Fix bug #8175 - smbd deadlock. Force the open operation (which is the expensive one anyway) to acquire and release locks in a way compatible with the more common do_lock check. This should fix the issue in the mundane, non-clusered case. Volker and Christian, you'll have to decide if this works well enough for you in the clustered case. Jeremy. --- source3/smbd/open.c | 134 +++++++++++++++++++++++++++++++------------------- 1 files changed, 83 insertions(+), 51 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index bb7e6c2..26ea209 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1055,18 +1055,8 @@ static bool delay_for_exclusive_oplocks(files_struct *fsp, return false; } -static bool file_has_brlocks(files_struct *fsp) -{ - struct byte_range_lock *br_lck; - - br_lck = brl_get_locks_readonly(fsp); - if (!br_lck) - return false; - - return br_lck->num_locks > 0 ? true : false; -} - static void grant_fsp_oplock_type(files_struct *fsp, + const struct byte_range_lock *br_lck, int oplock_request, bool got_level2_oplock, bool got_a_none_oplock) @@ -1084,7 +1074,7 @@ static void grant_fsp_oplock_type(files_struct *fsp, DEBUG(10,("grant_fsp_oplock_type: oplock type 0x%x on file %s\n", fsp->oplock_type, fsp_str_dbg(fsp))); return; - } else if (lp_locking(fsp->conn->params) && file_has_brlocks(fsp)) { + } else if (br_lck && br_lck->num_locks > 0) { DEBUG(10,("grant_fsp_oplock_type: file %s has byte range locks\n", fsp_str_dbg(fsp))); fsp->oplock_type = NO_OPLOCK; @@ -1563,6 +1553,50 @@ void remove_deferred_open_entry(struct file_id id, uint64_t mid, } /**************************************************************************** + Release/Acquire locks in order to prevent deadlocks. +****************************************************************************/ + +static void free_ordered_locks(struct share_mode_lock *lck, + struct byte_range_lock *br_lck) +{ + TALLOC_FREE(lck); + TALLOC_FREE(br_lck); +} + +static bool acquire_ordered_locks(TALLOC_CTX *mem_ctx, + files_struct *fsp, + const struct file_id id, + const char *connectpath, + const struct smb_filename *smb_fname, + const struct timespec *p_old_write_time, + struct share_mode_lock **p_lck, + struct byte_range_lock **p_br_lck) +{ + /* Ordering - we must get the br_lck for this + file before the share mode. */ + if (lp_locking(fsp->conn->params)) { + *p_br_lck = brl_get_locks(mem_ctx, fsp); + if (*p_br_lck == NULL) { + DEBUG(0, ("Could not get br_lock\n")); + return false; + } + } + + *p_lck = get_share_mode_lock(mem_ctx, + id, + connectpath, + smb_fname, + p_old_write_time); + + if (*p_lck == NULL) { + DEBUG(0, ("Could not get share mode lock\n")); + TALLOC_FREE(*p_br_lck); + return false; + } + return true; +} + +/**************************************************************************** Open a file with a share mode. Passed in an already created files_struct *. ****************************************************************************/ @@ -1595,6 +1629,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, uint32 existing_dos_attributes = 0; struct timeval request_time = timeval_zero(); struct share_mode_lock *lck = NULL; + struct byte_range_lock *br_lck = NULL; uint32 open_access_mask = access_mask; NTSTATUS status; char *parent_dir; @@ -1914,12 +1949,14 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, struct timespec old_write_time = smb_fname->st.st_ex_mtime; id = vfs_file_id_from_sbuf(conn, &smb_fname->st); - lck = get_share_mode_lock(talloc_tos(), id, - conn->connectpath, - smb_fname, &old_write_time); - - if (lck == NULL) { - DEBUG(0, ("Could not get share mode lock\n")); + if (!acquire_ordered_locks(talloc_tos(), + fsp, + id, + conn->connectpath, + smb_fname, + &old_write_time, + &lck, + &br_lck)) { return NT_STATUS_SHARING_VIOLATION; } @@ -1939,7 +1976,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, oplock_request, batch_entry)) { schedule_defer_open(lck, request_time, req); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); return NT_STATUS_SHARING_VIOLATION; } @@ -1961,18 +1998,19 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, oplock_request, exclusive_entry)) { schedule_defer_open(lck, request_time, req); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); return NT_STATUS_SHARING_VIOLATION; } } if (NT_STATUS_EQUAL(status, NT_STATUS_DELETE_PENDING)) { /* DELETE_PENDING is not deferred for a second */ - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); return status; } grant_fsp_oplock_type(fsp, + br_lck, oplock_request, got_level2_oplock, got_a_none_oplock); @@ -1991,7 +2029,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if (req == NULL) { DEBUG(0, ("DOS open without an SMB " "request!\n")); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); return NT_STATUS_INTERNAL_ERROR; } @@ -2009,7 +2047,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, create_options); if (NT_STATUS_IS_OK(status)) { - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); if (pinfo) { *pinfo = FILE_WAS_OPENED; } @@ -2085,7 +2123,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, } } - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); if (can_access) { /* * We have detected a sharing violation here @@ -2099,12 +2137,11 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, } /* - * We exit this block with the share entry *locked*..... + * We exit this block with the share entry and byte range + * entry *locked*..... */ } - SMB_ASSERT(!file_existed || (lck != NULL)); - /* * Ensure we pay attention to default ACLs on directories if required. */ @@ -2129,9 +2166,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, open_access_mask); if (!NT_STATUS_IS_OK(fsp_open)) { - if (lck != NULL) { - TALLOC_FREE(lck); - } + free_ordered_locks(lck, br_lck); return fsp_open; } @@ -2158,15 +2193,14 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, id = fsp->file_id; - lck = get_share_mode_lock(talloc_tos(), id, - conn->connectpath, - smb_fname, &old_write_time); - - if (lck == NULL) { - DEBUG(0, ("open_file_ntcreate: Could not get share " - "mode lock for %s\n", - smb_fname_str_dbg(smb_fname))); - fd_close(fsp); + if (!acquire_ordered_locks(talloc_tos(), + fsp, + id, + conn->connectpath, + smb_fname, + &old_write_time, + &lck, + &br_lck)) { return NT_STATUS_SHARING_VIOLATION; } @@ -2186,7 +2220,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, oplock_request, batch_entry)) { schedule_defer_open(lck, request_time, req); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return NT_STATUS_SHARING_VIOLATION; } @@ -2207,7 +2241,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, oplock_request, exclusive_entry)) { schedule_defer_open(lck, request_time, req); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return NT_STATUS_SHARING_VIOLATION; } @@ -2216,8 +2250,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if (!NT_STATUS_IS_OK(status)) { struct deferred_open_record state; - fd_close(fsp); - state.delayed_for_oplocks = False; state.id = id; @@ -2232,11 +2264,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, defer_open(lck, request_time, timeval_zero(), req, &state); } - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); + fd_close(fsp); return status; } grant_fsp_oplock_type(fsp, + br_lck, oplock_request, got_level2_oplock, got_a_none_oplock); @@ -2247,14 +2281,12 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, } - SMB_ASSERT(lck != NULL); - /* Delete streams if create_disposition requires it */ if (file_existed && clear_ads && !is_ntfs_stream_smb_fname(smb_fname)) { status = delete_all_streams(conn, smb_fname->base_name); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return status; } @@ -2273,7 +2305,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, ret_flock = SMB_VFS_KERNEL_FLOCK(fsp, share_access, access_mask); if(ret_flock == -1 ){ - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return NT_STATUS_SHARING_VIOLATION; @@ -2298,7 +2330,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if ((SMB_VFS_FTRUNCATE(fsp, 0) == -1) || (SMB_VFS_FSTAT(fsp, &smb_fname->st)==-1)) { status = map_nt_error_from_unix(errno); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return status; } @@ -2369,7 +2401,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if (!NT_STATUS_IS_OK(status)) { /* Remember to delete the mode we just added. */ del_share_mode(lck, fsp); - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); fd_close(fsp); return status; } @@ -2452,7 +2484,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, del_deferred_open_entry(lck, req->mid, sconn_server_id(req->sconn)); } - TALLOC_FREE(lck); + free_ordered_locks(lck, br_lck); return NT_STATUS_OK; } -- 1.7.3.1