From 84554f11d8e80720d61ca895c0a512c543a0986f Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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