[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Tue Jul 10 13:58:01 MDT 2012


The branch, master has been updated
       via  6d903bf Cope with a (non-security) open race we've had for ever as NTCreateX isn't atomic on POSIX.
       via  69a3e94 Now we have a guaranteed indication of a file being created, use it to set the create disposition correctly.
       via  02d42be Add function fd_open_atomic() which uses O_CREAT|O_EXCL to return a guaranteed indication of creation of a new file.
      from  3aa186f Simplify the logic in open_file() some more.

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


- Log -----------------------------------------------------------------
commit 6d903bf1899987adaeaaf6608ac318aca4588590
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jul 10 10:15:07 2012 -0700

    Cope with a (non-security) open race we've had for ever as NTCreateX isn't atomic on POSIX.
    
    On open without create, the file did exist, but some
    other (local or NFS) process either renamed/unlinked
    and re-created the file with different dev/ino after
    we walked the path, but before we did the open. We
    could retry the open but it's a rare enough case it's
    easier to just fail the open to prevent creating any
    problems in the open file db having the wrong dev/ino
    key.
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Tue Jul 10 21:57:33 CEST 2012 on sn-devel-104

commit 69a3e947b60397c9bb9175cf52fe009b6b057350
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Jul 9 17:03:45 2012 -0700

    Now we have a guaranteed indication of a file being created, use it to set the create disposition correctly.

commit 02d42be2589ff821ea9f63140694099d518f3046
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Jul 9 16:59:49 2012 -0700

    Add function fd_open_atomic() which uses O_CREAT|O_EXCL to return a guaranteed indication of creation of a new file.

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

Summary of changes:
 source3/smbd/open.c |  156 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 140 insertions(+), 16 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 473fd97..0f4a588 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -534,6 +534,107 @@ NTSTATUS change_dir_owner_to_parent(connection_struct *conn,
 }
 
 /****************************************************************************
+ Open a file - returning a guaranteed ATOMIC indication of if the
+ file was created or not.
+****************************************************************************/
+
+static NTSTATUS fd_open_atomic(struct connection_struct *conn,
+			files_struct *fsp,
+			int flags,
+			mode_t mode,
+			bool *file_created)
+{
+	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+	bool file_existed = VALID_STAT(fsp->fsp_name->st);
+
+	*file_created = false;
+
+	if (!(flags & O_CREAT)) {
+		/*
+		 * We're not creating the file, just pass through.
+		 */
+		return fd_open(conn, fsp, flags, mode);
+	}
+
+	if (flags & O_EXCL) {
+		/*
+		 * Fail if already exists, just pass through.
+		 */
+		status = fd_open(conn, fsp, flags, mode);
+		if (NT_STATUS_IS_OK(status)) {
+			/*
+			 * Here we've opened with O_CREAT|O_EXCL
+			 * and got success. We *know* we created
+			 * this file.
+			 */
+			*file_created = true;
+		}
+		return status;
+	}
+
+	/*
+	 * Now it gets tricky. We have O_CREAT, but not O_EXCL.
+	 * To know absolutely if we created the file or not,
+	 * we can never call O_CREAT without O_EXCL. So if
+	 * we think the file existed, try without O_CREAT|O_EXCL.
+	 * If we think the file didn't exist, try with
+	 * O_CREAT|O_EXCL. Keep bouncing between these two
+	 * requests until either the file is created, or
+	 * opened. Either way, we keep going until we get
+	 * a returnable result (error, or open/create).
+	 */
+
+	while(1) {
+		int curr_flags = flags;
+
+		if (file_existed) {
+			/* Just try open, do not create. */
+			curr_flags &= ~(O_CREAT);
+			status = fd_open(conn, fsp, curr_flags, mode);
+			if (NT_STATUS_EQUAL(status,
+					NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+				/*
+				 * Someone deleted it in the meantime.
+				 * Retry with O_EXCL.
+				 */
+				file_existed = false;
+				DEBUG(10,("fd_open_atomic: file %s existed. "
+					"Retry.\n",
+					smb_fname_str_dbg(fsp->fsp_name)));
+					continue;
+			}
+		} else {
+			/* Try create exclusively, fail if it exists. */
+			curr_flags |= O_EXCL;
+			status = fd_open(conn, fsp, curr_flags, mode);
+			if (NT_STATUS_EQUAL(status,
+					NT_STATUS_OBJECT_NAME_COLLISION)) {
+				/*
+				 * Someone created it in the meantime.
+				 * Retry without O_CREAT.
+				 */
+				file_existed = true;
+				DEBUG(10,("fd_open_atomic: file %s "
+					"did not exist. Retry.\n",
+					smb_fname_str_dbg(fsp->fsp_name)));
+				continue;
+			}
+			if (NT_STATUS_IS_OK(status)) {
+				/*
+				 * Here we've opened with O_CREAT|O_EXCL
+				 * and got success. We *know* we created
+				 * this file.
+				 */
+				*file_created = true;
+			}
+		}
+		/* Create is done, or failed. */
+		break;
+	}
+	return status;
+}
+
+/****************************************************************************
  Open a file.
 ****************************************************************************/
 
@@ -544,7 +645,8 @@ static NTSTATUS open_file(files_struct *fsp,
 			  int flags,
 			  mode_t unx_mode,
 			  uint32 access_mask, /* client requested access mask. */
-			  uint32 open_access_mask) /* what we're actually using in the open. */
+			  uint32 open_access_mask, /* what we're actually using in the open. */
+			  bool *p_file_created)
 {
 	struct smb_filename *smb_fname = fsp->fsp_name;
 	NTSTATUS status = NT_STATUS_OK;
@@ -670,7 +772,8 @@ static NTSTATUS open_file(files_struct *fsp,
 		}
 
 		/* Actually do the open */
-		status = fd_open(conn, fsp, local_flags, unx_mode);
+		status = fd_open_atomic(conn, fsp, local_flags,
+				unx_mode, p_file_created);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(3,("Error opening file %s (%s) (local_flags=%d) "
 				 "(flags=%d)\n", smb_fname_str_dbg(smb_fname),
@@ -690,7 +793,7 @@ static NTSTATUS open_file(files_struct *fsp,
 			return status;
 		}
 
-		if ((local_flags & O_CREAT) && !file_existed) {
+		if (*p_file_created) {
 			/* We created this file. */
 
 			bool need_re_stat = false;
@@ -1707,6 +1810,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	uint32 open_access_mask = access_mask;
 	NTSTATUS status;
 	char *parent_dir;
+	SMB_STRUCT_STAT saved_stat = smb_fname->st;
 
 	if (conn->printer) {
 		/*
@@ -2236,7 +2340,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 	fsp_open = open_file(fsp, conn, req, parent_dir,
 			     flags|flags2, unx_mode, access_mask,
-			     open_access_mask);
+			     open_access_mask, &new_file_created);
 
 	if (!NT_STATUS_IS_OK(fsp_open)) {
 		if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
@@ -2246,6 +2350,30 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		return fsp_open;
 	}
 
+	if (file_existed && !check_same_dev_ino(&saved_stat, &smb_fname->st)) {
+		/*
+		 * The file did exist, but some other (local or NFS)
+		 * process either renamed/unlinked and re-created the
+		 * file with different dev/ino after we walked the path,
+		 * but before we did the open. We could retry the
+		 * open but it's a rare enough case it's easier to
+		 * just fail the open to prevent creating any problems
+		 * in the open file db having the wrong dev/ino key.
+		 */
+		TALLOC_FREE(lck);
+		fd_close(fsp);
+		DEBUG(1,("open_file_ntcreate: file %s - dev/ino mismatch. "
+			"Old (dev=0x%llu, ino =0x%llu). "
+			"New (dev=0x%llu, ino=0x%llu). Failing open "
+			" with NT_STATUS_ACCESS_DENIED.\n",
+			 smb_fname_str_dbg(smb_fname),
+			 (unsigned long long)saved_stat.st_ex_dev,
+			 (unsigned long long)saved_stat.st_ex_ino,
+			 (unsigned long long)smb_fname->st.st_ex_dev,
+			 (unsigned long long)smb_fname->st.st_ex_ino));
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	if (!file_existed) {
 		struct share_mode_entry *batch_entry = NULL;
 		struct share_mode_entry *exclusive_entry = NULL;
@@ -2362,7 +2490,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	SMB_ASSERT(lck != NULL);
 
 	/* Delete streams if create_disposition requires it */
-	if (file_existed && clear_ads &&
+	if (!new_file_created && clear_ads &&
 	    !is_ntfs_stream_smb_fname(smb_fname)) {
 		status = delete_all_streams(conn, smb_fname->base_name);
 		if (!NT_STATUS_IS_OK(status)) {
@@ -2402,7 +2530,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	 * If requested, truncate the file.
 	 */
 
-	if (file_existed && (flags2&O_TRUNC)) {
+	if (!new_file_created && (flags2&O_TRUNC)) {
 		/*
 		 * We are modifying the file after open - update the stat
 		 * struct..
@@ -2438,14 +2566,16 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		if (is_stat_open(open_access_mask)) {
 			fsp->oplock_type = NO_OPLOCK;
 		}
+	}
 
+	if (new_file_created) {
+		info = FILE_WAS_CREATED;
+	} else {
 		if (flags2 & O_TRUNC) {
 			info = FILE_WAS_OVERWRITTEN;
 		} else {
 			info = FILE_WAS_OPENED;
 		}
-	} else {
-		info = FILE_WAS_CREATED;
 	}
 
 	if (pinfo) {
@@ -2487,13 +2617,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		fsp->initial_delete_on_close = True;
 	}
 
-	if (info == FILE_WAS_OVERWRITTEN
-	    || info == FILE_WAS_CREATED
-	    || info == FILE_WAS_SUPERSEDED) {
-		new_file_created = True;
-	}
-
-	if (new_file_created) {
+	if (info != FILE_WAS_OPENED) {
 		/* Files should be initially set as archive */
 		if (lp_map_archive(SNUM(conn)) ||
 		    lp_store_dos_attributes(SNUM(conn))) {
@@ -2521,7 +2645,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	 * selected.
 	 */
 
-	if (!posix_open && !file_existed && !def_acl) {
+	if (!posix_open && new_file_created && !def_acl) {
 
 		int saved_errno = errno; /* We might get ENOSYS in the next
 					  * call.. */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list