Attachment 8209 Details for Bug 9396 – Patchset for master.

[patch] Patchset for master.

bug-9396.master.patchset (text/plain), 13.79 KB, created by Jeremy Allison on 2012-11-16 19:48:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2012-11-16 19:48:46 UTC

Size: 13.79 KB

patch

obsolete

>From 84554f11d8e80720d61ca895c0a512c543a0986f Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:27:24 -0800
>Subject: [PATCH 1/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Make rename opens internal-only. They don't break oplocks.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/reply.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 8db9c62..f1c7478 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6597,7 +6597,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			FILE_OPEN,			/* create_disposition*/
> 			create_options,			/* create_options */
> 			posix_pathnames ? FILE_FLAG_POSIX_SEMANTICS|0777 : 0, /* file_attributes */
>-			0,				/* oplock_request */
>+			INTERNAL_OPEN_ONLY,		/* oplock_request */
> 			0,				/* allocation_size */
> 			0,				/* private_flags */
> 			NULL,				/* sd */
>@@ -6743,7 +6743,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			FILE_OPEN,			/* create_disposition*/
> 			create_options,			/* create_options */
> 			posix_pathnames ? FILE_FLAG_POSIX_SEMANTICS|0777 : 0, /* file_attributes */
>-			0,				/* oplock_request */
>+			INTERNAL_OPEN_ONLY,		/* oplock_request */
> 			0,				/* allocation_size */
> 			0,				/* private_flags */
> 			NULL,				/* sd */
>-- 
>1.7.7.3
>
>
>From 4483b825e18feccdadf5ec6d4d1da3ed0489a2a3 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:29:01 -0800
>Subject: [PATCH 2/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Rename opens should always only ask for DELETE_ACCESS. This makes the
>access_mask parameter redundent, next patch will change it to be a
>share_access parameter instead (which does vary between NTCreateX and
>SMBmv calls).
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/reply.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index f1c7478..d53b342 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6591,7 +6591,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			req,				/* req */
> 			0,				/* root_dir_fid */
> 			smb_fname_src,			/* fname */
>-			access_mask,			/* access_mask */
>+			DELETE_ACCESS,			/* access_mask */
> 			(FILE_SHARE_READ |		/* share_access */
> 			    FILE_SHARE_WRITE),
> 			FILE_OPEN,			/* create_disposition*/
>@@ -6737,7 +6737,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			req,				/* req */
> 			0,				/* root_dir_fid */
> 			smb_fname_src,			/* fname */
>-			access_mask,			/* access_mask */
>+			DELETE_ACCESS,			/* access_mask */
> 			(FILE_SHARE_READ |		/* share_access */
> 			    FILE_SHARE_WRITE),
> 			FILE_OPEN,			/* create_disposition*/
>-- 
>1.7.7.3
>
>
>From 2b64828d4ea6a3666116fdca2cfe1ff73445e354 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:41:10 -0800
>Subject: [PATCH 3/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Change the access_mask parameter in rename_internals() to a
>share_access parameter, and use the appropriate values in the
>different calls to rename_internals().
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/nttrans.c |    3 ++-
> source3/smbd/proto.h   |    2 +-
> source3/smbd/reply.c   |   12 ++++++------
> source3/smbd/trans2.c  |    4 +++-
> 4 files changed, 12 insertions(+), 9 deletions(-)
>
>diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
>index f5e5877..f0a1908 100644
>--- a/source3/smbd/nttrans.c
>+++ b/source3/smbd/nttrans.c
>@@ -1662,7 +1662,8 @@ void reply_ntrename(struct smb_request *req)
> 						  smb_fname_old, smb_fname_new,
> 						  attrs, False, src_has_wcard,
> 						  dest_has_wcard,
>-						  DELETE_ACCESS);
>+						  FILE_SHARE_READ|
>+						  FILE_SHARE_WRITE);
> 			break;
> 		case RENAME_FLAG_HARD_LINK:
> 			if (src_has_wcard || dest_has_wcard) {
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index f95fddd..7d405a6 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -904,7 +904,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			bool replace_if_exists,
> 			bool src_has_wild,
> 			bool dest_has_wild,
>-			uint32_t access_mask);
>+			uint32_t share_access);
> void reply_mv(struct smb_request *req);
> NTSTATUS copy_file(TALLOC_CTX *ctx,
> 			connection_struct *conn,
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index d53b342..69657b1 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6473,7 +6473,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			bool replace_if_exists,
> 			bool src_has_wild,
> 			bool dest_has_wild,
>-			uint32_t access_mask)
>+			uint32_t share_access)
> {
> 	char *fname_src_dir = NULL;
> 	char *fname_src_mask = NULL;
>@@ -6592,8 +6592,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			0,				/* root_dir_fid */
> 			smb_fname_src,			/* fname */
> 			DELETE_ACCESS,			/* access_mask */
>-			(FILE_SHARE_READ |		/* share_access */
>-			    FILE_SHARE_WRITE),
>+			share_access,			/* share_access */
> 			FILE_OPEN,			/* create_disposition*/
> 			create_options,			/* create_options */
> 			posix_pathnames ? FILE_FLAG_POSIX_SEMANTICS|0777 : 0, /* file_attributes */
>@@ -6738,8 +6737,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
> 			0,				/* root_dir_fid */
> 			smb_fname_src,			/* fname */
> 			DELETE_ACCESS,			/* access_mask */
>-			(FILE_SHARE_READ |		/* share_access */
>-			    FILE_SHARE_WRITE),
>+			share_access,			/* share_access */
> 			FILE_OPEN,			/* create_disposition*/
> 			create_options,			/* create_options */
> 			posix_pathnames ? FILE_FLAG_POSIX_SEMANTICS|0777 : 0, /* file_attributes */
>@@ -6910,7 +6908,9 @@ void reply_mv(struct smb_request *req)
> 
> 	status = rename_internals(ctx, conn, req, smb_fname_src, smb_fname_dst,
> 				  attrs, False, src_has_wcard, dest_has_wcard,
>-				  DELETE_ACCESS);
>+				  FILE_SHARE_READ|
>+					FILE_SHARE_WRITE|
>+					FILE_SHARE_DELETE);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		if (open_was_deferred(req->sconn, req->mid)) {
> 			/* We have re-scheduled this call. */
>diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
>index 61d755c..acd7a58 100644
>--- a/source3/smbd/trans2.c
>+++ b/source3/smbd/trans2.c
>@@ -6393,7 +6393,9 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn,
> 		status = rename_internals(ctx, conn, req, smb_fname_src,
> 					  smb_fname_dst, 0, overwrite, false,
> 					  dest_has_wcard,
>-					  FILE_WRITE_ATTRIBUTES);
>+					  FILE_SHARE_READ|
>+						FILE_SHARE_WRITE|
>+						FILE_SHARE_DELETE);
> 	}
>  out:
> 	TALLOC_FREE(smb_fname_dst);
>-- 
>1.7.7.3
>
>
>From 86e4fa08b28017fad5e62b714f3ea602c6270c27 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:43:22 -0800
>Subject: [PATCH 4/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Only check for DELETE_ACCESS in can_rename(). That's the only
>access value that matters.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/reply.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 69657b1..1235318 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -2566,7 +2566,7 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
> 		return NT_STATUS_OK;
> 	}
> 
>-	if (fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
>+	if (fsp->access_mask & DELETE_ACCESS) {
> 		return NT_STATUS_OK;
> 	}
> 
>-- 
>1.7.7.3
>
>
>From 98cac0723ddbeb2c7453a1dc8e994693c6e02010 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:47:56 -0800
>Subject: [PATCH 5/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Remove an unncessary error code mapping.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/reply.c |    2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 1235318..823637b 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6372,8 +6372,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
> 		DEBUG(3, ("rename_internals_fsp: Error %s rename %s -> %s\n",
> 			  nt_errstr(status), smb_fname_str_dbg(fsp->fsp_name),
> 			  smb_fname_str_dbg(smb_fname_dst)));
>-		if (NT_STATUS_EQUAL(status,NT_STATUS_SHARING_VIOLATION))
>-			status = NT_STATUS_ACCESS_DENIED;
> 		goto out;
> 	}
> 
>-- 
>1.7.7.3
>
>
>From eebcb22d37508ee104902893826b718235e9c25f Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:50:08 -0800
>Subject: [PATCH 6/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Make open_mode_check() a public function. We're going to need this
>inside parent_dirname_compatible_open() for rename checks.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c  |    2 +-
> source3/smbd/proto.h |    7 +++++++
> 2 files changed, 8 insertions(+), 1 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 7eb9f32..875ebb9 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -1085,7 +1085,7 @@ bool is_stat_open(uint32 access_mask)
>  Returns -1 on error, or number of share modes on success (may be zero).
> ****************************************************************************/
> 
>-static NTSTATUS open_mode_check(connection_struct *conn,
>+NTSTATUS open_mode_check(connection_struct *conn,
> 				struct share_mode_lock *lck,
> 				uint32_t name_hash,
> 				uint32 access_mask,
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index 7d405a6..42a806a 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -612,6 +612,13 @@ NTSTATUS change_dir_owner_to_parent(connection_struct *conn,
> 				    const char *fname,
> 				    SMB_STRUCT_STAT *psbuf);
> bool is_stat_open(uint32 access_mask);
>+NTSTATUS open_mode_check(connection_struct *conn,
>+			struct share_mode_lock *lck,
>+			uint32_t name_hash,
>+			uint32 access_mask,
>+			uint32 share_access,
>+			uint32 create_options,
>+			bool *file_existed);
> void remove_deferred_open_entry(struct file_id id, uint64_t mid,
> 				struct server_id pid);
> bool is_deferred_open_async(const void *ptr);
>-- 
>1.7.7.3
>
>
>From 0e9b97c467e3aa3456401db8d3f747f4b7b9f66a Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 10:57:23 -0800
>Subject: [PATCH 7/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>Change parent_dirname_compatible_open() to check the share mode
>table for the parent directory and reject incompatible opens.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/reply.c |   52 ++++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 42 insertions(+), 10 deletions(-)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 823637b..4108869 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6156,8 +6156,12 @@ static NTSTATUS parent_dirname_compatible_open(connection_struct *conn,
> 	char *parent_dir = NULL;
> 	struct smb_filename smb_fname_parent;
> 	struct file_id id;
>-	files_struct *fsp = NULL;
> 	int ret;
>+	struct share_mode_lock *lck = NULL;
>+	struct timespec *dummy = NULL;
>+	NTSTATUS status;
>+	uint32_t parent_name_hash = 0;
>+	bool existed = false;
> 
> 	if (!parent_dirname(talloc_tos(), smb_fname_dst_in->base_name,
> 			&parent_dir, NULL)) {
>@@ -6171,19 +6175,47 @@ static NTSTATUS parent_dirname_compatible_open(connection_struct *conn,
> 		return map_nt_error_from_unix(errno);
> 	}
> 
>+	status = file_name_hash(conn,
>+				smb_fname_parent.base_name,
>+				&parent_name_hash);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		return status;
>+	}
>+
> 	/*
>-	 * We're only checking on this smbd here, mostly good
>-	 * enough.. and will pass tests.
>+	 * Windows does a CreateFile on the parent
>+	 * directory in order to do a rename. We
>+	 * don't need to do that, but we do need to
>+	 * check the current open modes on the parent
>+	 * directory in order to pass all the rename
>+	 * tests in smbtorture.
> 	 */
> 
> 	id = vfs_file_id_from_sbuf(conn, &smb_fname_parent.st);
>-	for (fsp = file_find_di_first(conn->sconn, id); fsp;
>-			fsp = file_find_di_next(fsp)) {
>-		if (fsp->access_mask & DELETE_ACCESS) {
>-			return NT_STATUS_SHARING_VIOLATION;
>-                }
>-        }
>-	return NT_STATUS_OK;
>+	lck = get_share_mode_lock(talloc_tos(),
>+				id,
>+				conn->connectpath,
>+				&smb_fname_parent,
>+				dummy);
>+	if (!lck) {
>+		return NT_STATUS_OK;
>+	}
>+
>+	status = open_mode_check(conn,
>+				lck,
>+				parent_name_hash,
>+				FILE_WRITE_DATA,
>+				(FILE_SHARE_READ |
>+					FILE_SHARE_WRITE),
>+				0, /* create_options */
>+				&existed);
>+
>+	DEBUG(10,("open_mode_check on %s returned %s\n",
>+		smb_fname_str_dbg(&smb_fname_parent),
>+		nt_errstr(status) ));
>+
>+	TALLOC_FREE(lck);
>+	return status;
> }
> 
> /****************************************************************************
>-- 
>1.7.7.3
>
>
>From 3323a381f2460cd8e0af7565a3dceafb8dc8b501 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 16 Nov 2012 11:36:23 -0800
>Subject: [PATCH 8/8] Bug 9395 - Samba fails the simple_nodelete test of
> smb2.rename tests from master.
>
>We now pass all the smb2.rename tests, so remove them from knownfail.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail |    3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 953056e..00fd235 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -30,9 +30,6 @@
> ^samba3.*rap.sam.*.useradd # Not provided by Samba 3
> ^samba3.*rap.sam.*.userdelete # Not provided by Samba 3
> ^samba3.libsmbclient.opendir # This requires a workgroup called 'WORKGROUP' and for netbios browse lists to have been registered
>-# see bug 8412
>-^samba3.smb2.rename.*.simple_nodelete
>-^samba3.smb2.rename.*.no_share_delete_no_delete_access
> #These rpcclient combinations (pipe-level authentication but without sign or seal) need fixing in s3
> ^samba3.blackbox.rpcclient over ncacn_np with \[spnego\]
> ^samba3.blackbox.rpcclient over ncacn_np with \[spnego,bigendian\]
>-- 
>1.7.7.3
>

Actions: View

Attachments on bug 9396: 8209