The branch, master has been updated
via f0852a3 Remove locking across the lifetime of the copychunk call.
via f2d028e Move copychunk locking to be local to the read/write calls.
via d562e90 Add additional copychunk checks.
via d6e10f0 Move handle checking code to copychunk_check_handles().
from 7a21f60 tevent: Fix a comment
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit f0852a3483424d1242390011a65c427d3dbd80f2
Author: Jeremy Allison <j...@samba.org>
Date: Wed Jan 16 16:30:04 2013 -0800
Remove locking across the lifetime of the copychunk call.
Previous commit handles this around each read/write call.
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: David Disseldorp <dd...@samba.org>
Autobuild-User(master): David Disseldorp <dd...@samba.org>
Autobuild-Date(master): Fri Jan 18 01:47:01 CET 2013 on sn-devel-104
commit f2d028ef552adf13eed10b7db47e35bfa89a9c02
Author: Jeremy Allison <j...@samba.org>
Date: Wed Jan 16 16:29:11 2013 -0800
Move copychunk locking to be local to the read/write calls.
Eliminates the need to hold locks across the
entire lifetime of the call.
Next commit will remove these.
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: David Disseldorp <dd...@samba.org>
commit d562e9006a341ade6f38ee129598dd2e1dc3a493
Author: Jeremy Allison <j...@samba.org>
Date: Wed Jan 16 12:58:17 2013 -0800
Add additional copychunk checks.
For printer, ipc$ connections, and directory handles.
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: David Disseldorp <dd...@samba.org>
commit d6e10f00666b9baa17927067e58361ab39901fa1
Author: Jeremy Allison <j...@samba.org>
Date: Wed Jan 16 12:51:32 2013 -0800
Move handle checking code to copychunk_check_handles().
Planning to add extra checks to ensure we don't attempt
copychunk on printer or IPC$ handles.
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: David Disseldorp <dd...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/modules/vfs_default.c | 42 +++++++
source3/smbd/smb2_ioctl_network_fs.c | 197 ++++++++++-----------------------
2 files changed, 102 insertions(+), 137 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index d937c4a..8a03ea3 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1372,12 +1372,34 @@ static struct tevent_req
*vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
/* could use 2.6.33+ sendfile here to do this in kernel */
while (vfs_cc_state->copied < num) {
ssize_t ret;
+ struct lock_struct lck;
+ int saved_errno;
+
off_t this_num = MIN(sizeof(vfs_cc_state->buf),
num - vfs_cc_state->copied);
+ init_strict_lock_struct(src_fsp,
+ src_fsp->op->global->open_persistent_id,
+ src_off,
+ this_num,
+ READ_LOCK,
+ &lck);
+
+ if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &lck)) {
+ tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
+ return tevent_req_post(req, ev);
+ }
+
ret = SMB_VFS_PREAD(src_fsp, vfs_cc_state->buf,
this_num, src_off);
if (ret == -1) {
+ saved_errno = errno;
+ }
+
+ SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &lck);
+
+ if (ret == -1) {
+ errno = saved_errno;
tevent_req_nterror(req, map_nt_error_from_unix(errno));
return tevent_req_post(req, ev);
}
@@ -1386,11 +1408,31 @@ static struct tevent_req
*vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
return tevent_req_post(req, ev);
}
+
src_off += ret;
+ init_strict_lock_struct(dest_fsp,
+
dest_fsp->op->global->open_persistent_id,
+ dest_off,
+ this_num,
+ WRITE_LOCK,
+ &lck);
+
+ if (!SMB_VFS_STRICT_LOCK(dest_fsp->conn, dest_fsp, &lck)) {
+ tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
+ return tevent_req_post(req, ev);
+ }
+
ret = SMB_VFS_PWRITE(dest_fsp, vfs_cc_state->buf,
this_num, dest_off);
if (ret == -1) {
+ saved_errno = errno;
+ }
+
+ SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &lck);
+
+ if (ret == -1) {
+ errno = saved_errno;
tevent_req_nterror(req, map_nt_error_from_unix(errno));
return tevent_req_post(req, ev);
}
diff --git a/source3/smbd/smb2_ioctl_network_fs.c
b/source3/smbd/smb2_ioctl_network_fs.c
index 76625ab..a8d64e3 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -63,94 +63,6 @@ static NTSTATUS copychunk_check_limits(struct
srv_copychunk_copy *cc_copy)
return NT_STATUS_OK;
}
-static void copychunk_unlock_all(struct files_struct *src_fsp,
- struct files_struct *dst_fsp,
- struct lock_struct *rd_locks,
- struct lock_struct *wr_locks,
- uint32_t num_locks)
-{
-
- uint32_t i;
-
- for (i = 0; i < num_locks; i++) {
- SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &rd_locks[i]);
- SMB_VFS_STRICT_UNLOCK(dst_fsp->conn, dst_fsp, &wr_locks[i]);
- }
-}
-
-/* request read and write locks for each chunk */
-static NTSTATUS copychunk_lock_all(TALLOC_CTX *mem_ctx,
- struct srv_copychunk_copy *cc_copy,
- struct files_struct *src_fsp,
- struct files_struct *dst_fsp,
- struct lock_struct **rd_locks,
- struct lock_struct **wr_locks,
- uint32_t *num_locks)
-{
- NTSTATUS status;
- uint32_t i;
- struct lock_struct *rlocks;
- struct lock_struct *wlocks;
-
- rlocks = talloc_array(mem_ctx, struct lock_struct,
- cc_copy->chunk_count);
- if (rlocks == NULL) {
- status = NT_STATUS_NO_MEMORY;
- goto err_out;
- }
-
- wlocks = talloc_array(mem_ctx, struct lock_struct,
- cc_copy->chunk_count);
- if (wlocks == NULL) {
- status = NT_STATUS_NO_MEMORY;
- goto err_rlocks_free;
- }
-
- for (i = 0; i < cc_copy->chunk_count; i++) {
- init_strict_lock_struct(src_fsp,
- src_fsp->op->global->open_persistent_id,
- cc_copy->chunks[i].source_off,
- cc_copy->chunks[i].length,
- READ_LOCK,
- &rlocks[i]);
- init_strict_lock_struct(dst_fsp,
- dst_fsp->op->global->open_persistent_id,
- cc_copy->chunks[i].target_off,
- cc_copy->chunks[i].length,
- WRITE_LOCK,
- &wlocks[i]);
-
- if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &rlocks[i])) {
- status = NT_STATUS_FILE_LOCK_CONFLICT;
- goto err_unlock;
- }
- if (!SMB_VFS_STRICT_LOCK(dst_fsp->conn, dst_fsp, &wlocks[i])) {
- /* unlock last rlock, otherwise missed by cleanup */
- SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp,
- &rlocks[i]);
- status = NT_STATUS_FILE_LOCK_CONFLICT;
- goto err_unlock;
- }
- }
-
- *rd_locks = rlocks;
- *wr_locks = wlocks;
- *num_locks = i;
-
- return NT_STATUS_OK;
-
-err_unlock:
- if (i > 0) {
- /* cleanup all locks successfully issued so far */
- copychunk_unlock_all(src_fsp, dst_fsp, rlocks, wlocks, i);
- }
- talloc_free(wlocks);
-err_rlocks_free:
- talloc_free(rlocks);
-err_out:
- return status;
-}
-
struct fsctl_srv_copychunk_state {
struct connection_struct *conn;
uint32_t dispatch_count;
@@ -160,9 +72,6 @@ struct fsctl_srv_copychunk_state {
off_t total_written;
struct files_struct *src_fsp;
struct files_struct *dst_fsp;
- struct lock_struct *wlocks;
- struct lock_struct *rlocks;
- uint32_t num_locks;
enum {
COPYCHUNK_OUT_EMPTY = 0,
COPYCHUNK_OUT_LIMITS,
@@ -171,6 +80,60 @@ struct fsctl_srv_copychunk_state {
};
static void fsctl_srv_copychunk_vfs_done(struct tevent_req *subreq);
+static NTSTATUS copychunk_check_handles(struct files_struct *src_fsp,
+ struct files_struct *dst_fsp,
+ struct smb_request *smb1req)
+{
+ /*
+ * [MS-SMB2] 3.3.5.15.6 Handling a Server-Side Data Copy Request
+ * If Open.GrantedAccess of the destination file does not
+ * include FILE_WRITE_DATA, then the request MUST be failed with
+ * STATUS_ACCESS_DENIED. If Open.GrantedAccess of the
+ * destination file does not include FILE_READ_DATA access and
+ * the CtlCode is FSCTL_SRV_COPYCHUNK, then the request MUST be
+ * failed with STATUS_ACCESS_DENIED.
+ */
+ if (!CHECK_WRITE(dst_fsp)) {
+ DEBUG(5, ("copy chunk no write on dest handle (%s).\n",
+ smb_fname_str_dbg(dst_fsp->fsp_name) ));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ if (!CHECK_READ(dst_fsp, smb1req)) {
+ DEBUG(5, ("copy chunk no read on dest handle (%s).\n",
+ smb_fname_str_dbg(dst_fsp->fsp_name) ));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ if (!CHECK_READ(src_fsp, smb1req)) {
+ DEBUG(5, ("copy chunk no read on src handle (%s).\n",
+ smb_fname_str_dbg(src_fsp->fsp_name) ));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ if (src_fsp->is_directory) {
+ DEBUG(5, ("copy chunk no read on src directory handle (%s).\n",
+ smb_fname_str_dbg(src_fsp->fsp_name) ));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ if (dst_fsp->is_directory) {
+ DEBUG(5, ("copy chunk no read on dst directory handle (%s).\n",
+ smb_fname_str_dbg(dst_fsp->fsp_name) ));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ if (IS_IPC(src_fsp->conn) || IS_IPC(dst_fsp->conn)) {
+ DEBUG(5, ("copy chunk no access on IPC$ handle.\n"));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ if (IS_PRINT(src_fsp->conn) || IS_PRINT(dst_fsp->conn)) {
+ DEBUG(5, ("copy chunk no access on PRINT handle.\n"));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ return NT_STATUS_OK;
+}
+
static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct files_struct *dst_fsp,
@@ -224,27 +187,11 @@ static struct tevent_req
*fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
}
state->dst_fsp = dst_fsp;
- /*
- * [MS-SMB2] 3.3.5.15.6 Handling a Server-Side Data Copy Request
- * If Open.GrantedAccess of the destination file does not
- * include FILE_WRITE_DATA, then the request MUST be failed with
- * STATUS_ACCESS_DENIED. If Open.GrantedAccess of the
- * destination file does not include FILE_READ_DATA access and
- * the CtlCode is FSCTL_SRV_COPYCHUNK, then the request MUST be
- * failed with STATUS_ACCESS_DENIED.
- */
- if (!CHECK_WRITE(state->dst_fsp)) {
- state->status = NT_STATUS_ACCESS_DENIED;
- tevent_req_nterror(req, state->status);
- return tevent_req_post(req, ev);
- }
- if (!CHECK_READ(state->dst_fsp, smb2req->smb1req)) {
- state->status = NT_STATUS_ACCESS_DENIED;
- tevent_req_nterror(req, state->status);
- return tevent_req_post(req, ev);
- }
- if (!CHECK_READ(state->src_fsp, smb2req->smb1req)) {
- state->status = NT_STATUS_ACCESS_DENIED;
+
+ state->status = copychunk_check_handles(state->src_fsp,
+ state->dst_fsp,
+ smb2req->smb1req);
+ if (!NT_STATUS_IS_OK(state->status)) {
tevent_req_nterror(req, state->status);
return tevent_req_post(req, ev);
}
@@ -259,17 +206,6 @@ static struct tevent_req
*fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
/* any errors from here onwards should carry copychunk response data */
state->out_data = COPYCHUNK_OUT_RSP;
- state->status = copychunk_lock_all(state,
- &cc_copy,
- state->src_fsp,
- state->dst_fsp,
- &state->rlocks,
- &state->wlocks,
- &state->num_locks);
- if (tevent_req_nterror(req, state->status)) {
- return tevent_req_post(req, ev);
- }
-
for (i = 0; i < cc_copy.chunk_count; i++) {
struct tevent_req *vfs_subreq;
chunk = &cc_copy.chunks[i];
@@ -285,17 +221,12 @@ static struct tevent_req
*fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
state->status = NT_STATUS_NO_MEMORY;
if (state->dispatch_count == 0) {
/* nothing dispatched, return immediately */
- copychunk_unlock_all(state->src_fsp,
- state->dst_fsp,
- state->rlocks,
- state->wlocks,
- state->num_locks);
tevent_req_nterror(req, state->status);
return tevent_req_post(req, ev);
} else {
/*
* wait for dispatched to complete before
- * returning error, locks held.
+ * returning error.
*/
break;
}
@@ -305,7 +236,6 @@ static struct tevent_req
*fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
state->dispatch_count++;
}
- /* hold locks until all dispatched requests are completed */
return req;
}
@@ -345,13 +275,6 @@ static void fsctl_srv_copychunk_vfs_done(struct tevent_req
*subreq)
return;
}
- /* all VFS copy_chunk requests done */
- copychunk_unlock_all(state->src_fsp,
- state->dst_fsp,
- state->rlocks,
- state->wlocks,
- state->num_locks);
-
if (!tevent_req_nterror(req, state->status)) {
tevent_req_done(req);
}
--
Samba Shared Repository
