[SCM] Samba Shared Repository - branch master updated - 7b9f6dda131f471ae61c12e7eb06d67b8f02b1cf

Tim Prouty tprouty at samba.org
Wed Dec 10 02:57:00 GMT 2008


The branch, master has been updated
       via  7b9f6dda131f471ae61c12e7eb06d67b8f02b1cf (commit)
       via  4a9b092eb43603bae6190b8a5fdee20c9ebae26c (commit)
       via  a5651848b26719b7f9c06fbc996a369a5d97461d (commit)
      from  6e4cc12604f6bcf53961326d497f118dfe5da139 (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 7b9f6dda131f471ae61c12e7eb06d67b8f02b1cf
Author: Tim Prouty <tprouty at samba.org>
Date:   Sat Dec 6 16:08:35 2008 -0800

    s3: [3/3]: Fix a delete on close divergence from windows and the associated torture test
    
    This third patch cleans up by removing all of the code that is made
    obsolete by the first patch.  It should cause no functional changes.

commit 4a9b092eb43603bae6190b8a5fdee20c9ebae26c
Author: Tim Prouty <tprouty at samba.org>
Date:   Sun Dec 7 10:34:37 2008 -0800

    s4: [2/3] Fix a delete on close divergence from windows and the associated torture test
    
    This second patch fixes the deltest17 BASE-DELETE torture test to pass
    against win2k3/win2k8/winXPsp2

commit a5651848b26719b7f9c06fbc996a369a5d97461d
Author: Tim Prouty <tprouty at samba.org>
Date:   Sun Dec 7 10:30:01 2008 -0800

    s3: [1/3] Fix a delete on close divergence from windows and the associated torture test
    
    smbtorture4's BASE-DELETE:deltest17 was failing against win2k8,
    win2k3, and winXPsp2 but passing against samba.
    
    deltest17 does the following:
    
    1. open file -> file is created
    2. closes file
    3. open file with DOC -> fnum1
    4. check that DOC is not reported as being set from fnum1
    5. opens file again Read Only -> fnum2
    6. check that DOC is not reported as being set from either file handle
    7. close fnum1 (the file handle that requested DOC to be set)
    8. check if DOC is reported as being set from fnum2
     * This is where windows and samba begin to diverge.  Windows
       reports that the DOC bit is set, while samba reports that it is not set.
    9. close fnum2 (the last remaining open handle for the file)
    10.See if the file has been deleted.
     * On samba the file still exists.  On windows the file was deleted.
    
    The way open_file_ntcreate is written now, if an open has the DOC bit
    set on the wire, DOC (fsp->initial_delete_on_close) is not set unless:
    a. the open creates the file, or b. there is an open file handle with
    a share_entry in the struct lck that has the
    SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE bit set (let's call it
    SM_AIDOC).
    
    My understanding of SM_AIDOC is that it was added to differentiate
    between DOC being set on an open that creates a file vs an open that
    opens an existing.  As described in step 8/10 above, it appears that
    windows does not make this differentiation.
    
    To resolve this issue there are three patches.  This first patch is a
    simple proof of concept change that is sufficient to fix the bug.  It
    removes the differentiation in open_file_ntcreate, and updates
    deltest17 to allow it to pass against win2k3/xp.  This makes
    open_file_ntcreate more closely match the semantics in open_directory
    and rename_internals_fsp.  This change also does not break any other
    tests in BASE-DELETE or "make test".  Specifically test deltest20b
    which verifies the CIFSFS rename DOC semantics still passes :).

-----------------------------------------------------------------------

Summary of changes:
 source3/include/proto.h        |    4 +--
 source3/include/smb.h          |    1 -
 source3/locking/locking.c      |   46 +---------------------------------------
 source3/modules/onefs_open.c   |    5 +--
 source3/smbd/open.c            |   11 +++------
 source3/smbd/reply.c           |    2 -
 source4/torture/basic/delete.c |   23 +++++++++++++++----
 7 files changed, 26 insertions(+), 66 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/include/proto.h b/source3/include/proto.h
index d0ad361..2ba01b3 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -3455,7 +3455,7 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e);
 bool is_deferred_open_entry(const struct share_mode_entry *e);
 bool is_unused_share_mode_entry(const struct share_mode_entry *e);
 void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
-			uid_t uid, uint16 mid, uint16 op_type, bool initial_delete_on_close_allowed);
+		    uid_t uid, uint16 mid, uint16 op_type);
 void add_deferred_open(struct share_mode_lock *lck, uint16 mid,
 		       struct timeval request_time,
 		       struct file_id id);
@@ -3465,11 +3465,9 @@ bool remove_share_oplock(struct share_mode_lock *lck, files_struct *fsp);
 bool downgrade_share_oplock(struct share_mode_lock *lck, files_struct *fsp);
 NTSTATUS can_set_delete_on_close(files_struct *fsp, bool delete_on_close,
 				 uint32 dosmode);
-bool can_set_initial_delete_on_close(const struct share_mode_lock *lck);
 void set_delete_on_close_token(struct share_mode_lock *lck, UNIX_USER_TOKEN *tok);
 void set_delete_on_close_lck(struct share_mode_lock *lck, bool delete_on_close, UNIX_USER_TOKEN *tok);
 bool set_delete_on_close(files_struct *fsp, bool delete_on_close, UNIX_USER_TOKEN *tok);
-bool set_allow_initial_delete_on_close(struct share_mode_lock *lck, files_struct *fsp, bool delete_on_close);
 bool set_sticky_write_time(struct file_id fileid, struct timespec write_time);
 bool set_write_time(struct file_id fileid, struct timespec write_time);
 int share_mode_forall(void (*fn)(const struct share_mode_entry *, const char *,
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 39673e1..112b4e0 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -710,7 +710,6 @@ struct pending_message_list {
 };
 
 #define SHARE_MODE_FLAG_POSIX_OPEN	0x1
-#define SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE      0x2
 
 /* struct returned by get_share_modes */
 struct share_mode_entry {
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 33717f1..a70f9d2 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -1067,13 +1067,10 @@ static void add_share_mode_entry(struct share_mode_lock *lck,
 }
 
 void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
-			uid_t uid, uint16 mid, uint16 op_type, bool initial_delete_on_close_allowed)
+		    uid_t uid, uint16 mid, uint16 op_type)
 {
 	struct share_mode_entry entry;
 	fill_share_mode_entry(&entry, fsp, uid, mid, op_type);
-	if (initial_delete_on_close_allowed) {
-		entry.flags |= SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE;
-	}
 	add_share_mode_entry(lck, &entry);
 }
 
@@ -1271,22 +1268,6 @@ NTSTATUS can_set_delete_on_close(files_struct *fsp, bool delete_on_close,
 	return NT_STATUS_OK;
 }
 
-/****************************************************************************
- Do we have an open file handle that created this entry ?
-****************************************************************************/
-
-bool can_set_initial_delete_on_close(const struct share_mode_lock *lck)
-{
-	int i;
-
-	for (i=0; i<lck->num_share_modes; i++) {
-		if (lck->share_modes[i].flags & SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE) {
-			return True;
-		}
-	}
-	return False;
-}
-
 /*************************************************************************
  Return a talloced copy of a UNIX_USER_TOKEN. NULL on fail.
  (Should this be in locking.c.... ?).
@@ -1380,31 +1361,6 @@ bool set_delete_on_close(files_struct *fsp, bool delete_on_close, UNIX_USER_TOKE
 	return True;
 }
 
-/****************************************************************************
- Sets the allow initial delete on close flag for this share mode.
-****************************************************************************/
-
-bool set_allow_initial_delete_on_close(struct share_mode_lock *lck, files_struct *fsp, bool delete_on_close)
-{
-	struct share_mode_entry entry, *e;
-
-	/* Don't care about the pid owner being correct here - just a search. */
-	fill_share_mode_entry(&entry, fsp, (uid_t)-1, 0, NO_OPLOCK);
-
-	e = find_share_mode_entry(lck, &entry);
-	if (e == NULL) {
-		return False;
-	}
-
-	if (delete_on_close) {
-		e->flags |= SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE;
-	} else {
-		e->flags &= ~SHARE_MODE_ALLOW_INITIAL_DELETE_ON_CLOSE;
-	}
-	lck->modified = True;
-	return True;
-}
-
 bool set_sticky_write_time(struct file_id fileid, struct timespec write_time)
 {
 	struct share_mode_lock *lck;
diff --git a/source3/modules/onefs_open.c b/source3/modules/onefs_open.c
index bda5e7e..d0310d0 100644
--- a/source3/modules/onefs_open.c
+++ b/source3/modules/onefs_open.c
@@ -1182,7 +1182,7 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 	}
 
 	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0,
-		       fsp->oplock_type, true);
+		       fsp->oplock_type);
 
 	/* Handle strange delete on close create semantics. */
 	if (create_options & FILE_DELETE_ON_CLOSE) {
@@ -1521,8 +1521,7 @@ static NTSTATUS onefs_open_directory(connection_struct *conn,
 		return NT_STATUS_DELETE_PENDING;
 	}
 
-	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0, NO_OPLOCK,
-		       true);
+	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0, NO_OPLOCK);
 
 	/*
 	 * For directories the delete on close bit at open time seems
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 078b47a..77ad166 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1981,13 +1981,11 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	}
 
 	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0,
-		       fsp->oplock_type, new_file_created);
+		       fsp->oplock_type);
 
 	/* Handle strange delete on close create semantics. */
-	if ((create_options & FILE_DELETE_ON_CLOSE)
-	    && (((conn->fs_capabilities & FILE_NAMED_STREAMS)
-			&& is_ntfs_stream_name(fname))
-		|| can_set_initial_delete_on_close(lck))) {
+	if (create_options & FILE_DELETE_ON_CLOSE) {
+
 		status = can_set_delete_on_close(fsp, True, new_dos_attributes);
 
 		if (!NT_STATUS_IS_OK(status)) {
@@ -2421,8 +2419,7 @@ static NTSTATUS open_directory(connection_struct *conn,
 		return status;
 	}
 
-	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0, NO_OPLOCK,
-		       True);
+	set_share_mode(lck, fsp, conn->server_info->utok.uid, 0, NO_OPLOCK);
 
 	/* For directories the delete on close bit at open time seems
 	   always to be honored on close... See test 19 in Samba4 BASE-DELETE. */
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 00c744c..9f7a189 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -5583,8 +5583,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 		 * depends on these semantics. JRA.
 		 */
 
-		set_allow_initial_delete_on_close(lck, fsp, True);
-
 		if (create_options & FILE_DELETE_ON_CLOSE) {
 			status = can_set_delete_on_close(fsp, True, 0);
 
diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c
index b71c85a..c1ac62f 100644
--- a/source4/torture/basic/delete.c
+++ b/source4/torture/basic/delete.c
@@ -950,15 +950,18 @@ static bool deltest17(struct torture_context *tctx, struct smbcli_state *cli1, s
 
 	smbcli_close(cli1->tree, fnum1);
 
-	correct &= check_delete_on_close(tctx, cli1, fnum2, fname, false, __location__);
+	/* After the first close, the files has the delete on close bit set. */
+	correct &= check_delete_on_close(tctx, cli1, fnum2, fname, true, __location__);
 
 	smbcli_close(cli1->tree, fnum2);
 
-	/* See if the file is deleted - shouldn't be.... */
+	/* Make sure the file has been deleted */
 	fnum1 = smbcli_open(cli1->tree, fname, O_RDWR, DENY_NONE);
-	torture_assert(tctx, fnum1 != -1, talloc_asprintf(tctx, "open of %s failed (should succeed) - %s", 
+	torture_assert(tctx, fnum1 == -1, talloc_asprintf(tctx, "open of %s failed (should succeed) - %s",
 		       fname, smbcli_errstr(cli1->tree)));
 
+	CHECK_STATUS(cli1, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+
 	return correct;
 }
 
@@ -994,7 +997,12 @@ static bool deltest18(struct torture_context *tctx, struct smbcli_state *cli1, s
 	torture_assert(tctx, fnum1 != -1, talloc_asprintf(tctx, "open - 1 of %s failed (%s)", 
 		       dname, smbcli_errstr(cli1->tree)));
 
-	/* The delete on close bit is *not* reported as being set. */
+	/*
+	 * The delete on close bit is *not* reported as being set.
+	 * Win2k3/win2k8 should pass this check, but WinXPsp2 reports delete on
+	 * close as being set.  This causes the subsequent create to fail with
+	 * NT_STATUS_DELETE_PENDING.
+	 */
 	correct &= check_delete_on_close(tctx, cli1, fnum1, dname, false, __location__);
 
 	/* Now try opening again for read-only. */
@@ -1082,7 +1090,12 @@ static bool deltest19(struct torture_context *tctx, struct smbcli_state *cli1, s
 	torture_assert(tctx, fnum1 != -1, 
 		talloc_asprintf(tctx, "open - 1 of %s failed (%s)", fname, smbcli_errstr(cli1->tree)));
 
-	/* The delete on close bit is *not* reported as being set. */
+	/*
+	 * The delete on close bit is *not* reported as being set.
+	 * Win2k3/win2k8 should pass this check, but WinXPsp2 reports delete on
+	 * close as being set.  This causes the subsequent create to fail with
+	 * NT_STATUS_DELETE_PENDING.
+	 */
 	correct &= check_delete_on_close(tctx, cli1, fnum1, dname, false, __location__);
 
 	/* Now try opening again for read-only. */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list