First part of fix for bug #5903 - vfs_streams_xattr breaks contents of the file.

Restructures parts of open code so that fsp must be allocated before calling
open_file_ntcreate(_internal). Also fix up file ref-counting inside files.c.
Jeremy.
(cherry picked from commit 1f36db34d7483614e1a4d6d5f3437205bcb20541)

6 files changed, 120 insertions, 116 deletions

diff --git a/source/printing/printfsp.c b/source/printing/printfsp.c
index 1fde16b802d..c6652249278 100644
--- a/source/printing/printfsp.c
+++ b/source/printing/printfsp.c
@@ -28,19 +28,13 @@ print_job_start().
 ***************************************************************************/
 
 NTSTATUS print_fsp_open(connection_struct *conn, const char *fname,
-			files_struct **result)
+			files_struct *fsp)
 {
 	int jobid;
 	SMB_STRUCT_STAT sbuf;
-	files_struct *fsp;
 	fstring name;
 	NTSTATUS status;
 
-	status = file_new(conn, &fsp);
-	if(!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
 	fstrcpy( name, "Remote Downlevel Document");
 	if (fname) {
 		const char *p = strrchr(fname, '/');
@@ -54,7 +48,6 @@ NTSTATUS print_fsp_open(connection_struct *conn, const char *fname,
 	jobid = print_job_start(&current_user, SNUM(conn), name, NULL);
 	if (jobid == -1) {
 		status = map_nt_error_from_unix(errno);
-		file_free(fsp);
 		return status;
 	}
 
@@ -63,7 +56,6 @@ NTSTATUS print_fsp_open(connection_struct *conn, const char *fname,
 	if (fsp->rap_print_jobid == 0) {
 		/* We need to delete the entry in the tdb. */
 		pjob_delete(lp_const_servicename(SNUM(conn)), jobid);
-		file_free(fsp);
 		return NT_STATUS_ACCESS_DENIED;	/* No errno around here */
 	}
 
@@ -82,14 +74,11 @@ NTSTATUS print_fsp_open(connection_struct *conn, const char *fname,
 	fsp->sent_oplock_break = NO_BREAK_SENT;
 	fsp->is_directory = False;
 	string_set(&fsp->fsp_name,print_job_fname(lp_const_servicename(SNUM(conn)),jobid));
-	fsp->wcp = NULL; 
+	fsp->wcp = NULL;
 	SMB_VFS_FSTAT(fsp, &sbuf);
 	fsp->mode = sbuf.st_mode;
 	fsp->file_id = vfs_file_id_from_sbuf(conn, &sbuf);
 
-	conn->num_files_open++;
-
-	*result = fsp;
 	return NT_STATUS_OK;
 }
 
diff --git a/source/smbd/close.c b/source/smbd/close.c
index 3d3c4cf09e1..8c0fa99e1f9 100644
--- a/source/smbd/close.c
+++ b/source/smbd/close.c
@@ -106,8 +106,7 @@ static void check_magic(struct files_struct *fsp)
 static NTSTATUS close_filestruct(files_struct *fsp)
 {
 	NTSTATUS status = NT_STATUS_OK;
-	connection_struct *conn = fsp->conn;
-    
+
 	if (fsp->fh->fd != -1) {
 		if(flush_write_cache(fsp, CLOSE_FLUSH) == -1) {
 			status = map_nt_error_from_unix(errno);
@@ -115,9 +114,8 @@ static NTSTATUS close_filestruct(files_struct *fsp)
 		delete_write_cache(fsp);
 	}
 
-	conn->num_files_open--;
 	return status;
-}    
+}
 
 /****************************************************************************
  If any deferred opens are waiting on this close, notify them.
@@ -582,7 +580,7 @@ static NTSTATUS close_normal_file(files_struct *fsp, enum file_close_type close_
 
 	DEBUG(2,("%s closed file %s (numopen=%d) %s\n",
 		conn->user,fsp->fsp_name,
-		conn->num_files_open,
+		conn->num_files_open - 1,
 		nt_errstr(status) ));
 
 	file_free(fsp);
diff --git a/source/smbd/fake_file.c b/source/smbd/fake_file.c
index 31fe030f465..0f571ac2822 100644
--- a/source/smbd/fake_file.c
+++ b/source/smbd/fake_file.c
@@ -141,7 +141,6 @@ NTSTATUS open_fake_file(connection_struct *conn,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	conn->num_files_open++;
 	*result = fsp;
 	return NT_STATUS_OK;
 }
diff --git a/source/smbd/files.c b/source/smbd/files.c
index b7ac58d5232..8ed8eaf4f38 100644
--- a/source/smbd/files.c
+++ b/source/smbd/files.c
@@ -129,6 +129,8 @@ NTSTATUS file_new(connection_struct *conn, files_struct **result)
 
 	ZERO_STRUCT(fsp_fi_cache);
 
+	conn->num_files_open++;
+
 	*result = fsp;
 	return NT_STATUS_OK;
 }
@@ -427,9 +429,7 @@ void file_free(files_struct *fsp)
 	DEBUG(5,("freed files structure %d (%d used)\n",
 		 fsp->fnum, files_used));
 
-	/* this is paranoia, just in case someone tries to reuse the 
-	   information */
-	ZERO_STRUCTP(fsp);
+	fsp->conn->num_files_open--;
 
 	if (fsp == chain_fsp) {
 		chain_fsp = NULL;
@@ -445,6 +445,10 @@ void file_free(files_struct *fsp)
 		vfs_remove_fsp_extension(fsp->vfs_extension->owner, fsp);
 	}
 
+	/* this is paranoia, just in case someone tries to reuse the
+	   information */
+	ZERO_STRUCTP(fsp);
+
 	SAFE_FREE(fsp);
 }
 
@@ -500,49 +504,37 @@ void file_chain_reset(void)
  Duplicate the file handle part for a DOS or FCB open.
 ****************************************************************************/
 
-NTSTATUS dup_file_fsp(files_struct *fsp,
-				uint32 access_mask,
-				uint32 share_access,
-				uint32 create_options,
-		      		files_struct **result)
+void dup_file_fsp(files_struct *from,
+			uint32 access_mask,
+			uint32 share_access,
+			uint32 create_options,
+	      		files_struct *to)
 {
-	NTSTATUS status;
-	files_struct *dup_fsp;
-
-	status = file_new(fsp->conn, &dup_fsp);
-
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
-	SAFE_FREE(dup_fsp->fh);
-
-	dup_fsp->fh = fsp->fh;
-	dup_fsp->fh->ref_count++;
-
-	dup_fsp->file_id = fsp->file_id;
-	dup_fsp->initial_allocation_size = fsp->initial_allocation_size;
-	dup_fsp->mode = fsp->mode;
-	dup_fsp->file_pid = fsp->file_pid;
-	dup_fsp->vuid = fsp->vuid;
-	dup_fsp->open_time = fsp->open_time;
-	dup_fsp->access_mask = access_mask;
-	dup_fsp->share_access = share_access;
-	dup_fsp->oplock_type = fsp->oplock_type;
-	dup_fsp->can_lock = fsp->can_lock;
-	dup_fsp->can_read = (access_mask & (FILE_READ_DATA)) ? True : False;
-	if (!CAN_WRITE(fsp->conn)) {
-		dup_fsp->can_write = False;
+	SAFE_FREE(to->fh);
+
+	to->fh = from->fh;
+	to->fh->ref_count++;
+
+	to->file_id = from->file_id;
+	to->initial_allocation_size = from->initial_allocation_size;
+	to->mode = from->mode;
+	to->file_pid = from->file_pid;
+	to->vuid = from->vuid;
+	to->open_time = from->open_time;
+	to->access_mask = access_mask;
+	to->share_access = share_access;
+	to->oplock_type = from->oplock_type;
+	to->can_lock = from->can_lock;
+	to->can_read = (access_mask & (FILE_READ_DATA)) ? True : False;
+	if (!CAN_WRITE(from->conn)) {
+		to->can_write = False;
 	} else {
-		dup_fsp->can_write = (access_mask & (FILE_WRITE_DATA | FILE_APPEND_DATA)) ? True : False;
+		to->can_write = (access_mask & (FILE_WRITE_DATA | FILE_APPEND_DATA)) ? True : False;
 	}
-	dup_fsp->print_file = fsp->print_file;
-	dup_fsp->modified = fsp->modified;
-	dup_fsp->is_directory = fsp->is_directory;
-	dup_fsp->is_stat = fsp->is_stat;
-	dup_fsp->aio_write_behind = fsp->aio_write_behind;
-        string_set(&dup_fsp->fsp_name,fsp->fsp_name);
-
-	*result = dup_fsp;
-	return NT_STATUS_OK;
+	to->print_file = from->print_file;
+	to->modified = from->modified;
+	to->is_directory = from->is_directory;
+	to->is_stat = from->is_stat;
+	to->aio_write_behind = from->aio_write_behind;
+        string_set(&to->fsp_name,from->fsp_name);
 }
diff --git a/source/smbd/open.c b/source/smbd/open.c
index c21ad20312f..6ba6df2bc20 100644
--- a/source/smbd/open.c
+++ b/source/smbd/open.c
@@ -404,7 +404,7 @@ static NTSTATUS open_file(files_struct *fsp,
 		 *current_user_info.smb_name ?
 		 current_user_info.smb_name : conn->user,fsp->fsp_name,
 		 BOOLSTR(fsp->can_read), BOOLSTR(fsp->can_write),
-		 conn->num_files_open + 1));
+		 conn->num_files_open));
 
 	errno = 0;
 	return NT_STATUS_OK;
@@ -893,7 +893,8 @@ static bool open_match_attributes(connection_struct *conn,
  Try and find a duplicated file handle.
 ****************************************************************************/
 
-static files_struct *fcb_or_dos_open(connection_struct *conn,
+static NTSTATUS fcb_or_dos_open(connection_struct *conn,
+				     files_struct *fsp_to_dup_into,
 				     const char *fname, 
 				     struct file_id id,
 				     uint16 file_pid,
@@ -903,7 +904,6 @@ static files_struct *fcb_or_dos_open(connection_struct *conn,
 				     uint32 create_options)
 {
 	files_struct *fsp;
-	files_struct *dup_fsp;
 
 	DEBUG(5,("fcb_or_dos_open: attempting old open semantics for "
 		 "file %s.\n", fname ));
@@ -932,23 +932,21 @@ static files_struct *fcb_or_dos_open(connection_struct *conn,
 	}
 
 	if (!fsp) {
-		return NULL;
+		return NT_STATUS_NOT_FOUND;
 	}
 
 	/* quite an insane set of semantics ... */
 	if (is_executable(fname) &&
 	    (fsp->fh->private_options & NTCREATEX_OPTIONS_PRIVATE_DENY_DOS)) {
 		DEBUG(10,("fcb_or_dos_open: file fail due to is_executable.\n"));
-		return NULL;
+		return NT_STATUS_INVALID_PARAMETER;
 	}
 
 	/* We need to duplicate this fsp. */
-	if (!NT_STATUS_IS_OK(dup_file_fsp(fsp, access_mask, share_access,
-					  create_options, &dup_fsp))) {
-		return NULL;
-	}
+	dup_file_fsp(fsp, access_mask, share_access,
+			  create_options, fsp_to_dup_into);
 
-	return dup_fsp;
+	return NT_STATUS_OK;
 }
 
 /****************************************************************************
@@ -1128,10 +1126,10 @@ static void schedule_defer_open(struct share_mode_lock *lck,
 }
 
 /****************************************************************************
- Open a file with a share mode.
+ Open a file with a share mode. Passed in an already created files_struct *.
 ****************************************************************************/
 
-NTSTATUS open_file_ntcreate(connection_struct *conn,
+static NTSTATUS open_file_ntcreate_internal(connection_struct *conn,
 			    struct smb_request *req,
 			    const char *fname,
 			    SMB_STRUCT_STAT *psbuf,
@@ -1143,7 +1141,7 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			    int oplock_request, 	/* internal Samba oplock codes. */
 				 			/* Information (FILE_EXISTS etc.) */
 			    int *pinfo,
-			    files_struct **result)
+			    files_struct *fsp)
 {
 	int flags=0;
 	int flags2=0;
@@ -1153,7 +1151,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 	bool new_file_created = False;
 	struct file_id id;
 	NTSTATUS fsp_open = NT_STATUS_ACCESS_DENIED;
-	files_struct *fsp = NULL;
 	mode_t new_unx_mode = (mode_t)0;
 	mode_t unx_mode = (mode_t)0;
 	int info;
@@ -1170,7 +1167,7 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 	ZERO_STRUCT(id);
 
 	if (conn->printer) {
-		/* 
+		/*
 		 * Printers are handled completely differently.
 		 * Most of the passed parameters are ignored.
 		 */
@@ -1181,7 +1178,7 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 		DEBUG(10, ("open_file_ntcreate: printer open fname=%s\n", fname));
 
-		return print_fsp_open(conn, fname, result);
+		return print_fsp_open(conn, fname, fsp);
 	}
 
 	if (!parent_dirname_talloc(talloc_tos(), fname, &parent_dir,
@@ -1438,11 +1435,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	status = file_new(conn, &fsp);
-	if(!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
 	fsp->file_id = vfs_file_id_from_sbuf(conn, psbuf);
 	fsp->share_access = share_access;
 	fsp->fh->private_options = create_options;
@@ -1467,7 +1459,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 					  fname, &old_write_time);
 
 		if (lck == NULL) {
-			file_free(fsp);
 			DEBUG(0, ("Could not get share mode lock\n"));
 			return NT_STATUS_SHARING_VIOLATION;
 		}
@@ -1478,7 +1469,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 					 oplock_request)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
-			file_free(fsp);
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 
@@ -1498,7 +1488,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 						  oplock_request)) {
 				schedule_defer_open(lck, request_time, req);
 				TALLOC_FREE(lck);
-				file_free(fsp);
 				return NT_STATUS_SHARING_VIOLATION;
 			}
 		}
@@ -1506,7 +1495,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		if (NT_STATUS_EQUAL(status, NT_STATUS_DELETE_PENDING)) {
 			/* DELETE_PENDING is not deferred for a second */
 			TALLOC_FREE(lck);
-			file_free(fsp);
 			return status;
 		}
 
@@ -1521,33 +1509,30 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			if (create_options &
 			    (NTCREATEX_OPTIONS_PRIVATE_DENY_DOS|
 			     NTCREATEX_OPTIONS_PRIVATE_DENY_FCB)) {
-				files_struct *fsp_dup;
-
 				if (req == NULL) {
 					DEBUG(0, ("DOS open without an SMB "
 						  "request!\n"));
 					TALLOC_FREE(lck);
-					file_free(fsp);
 					return NT_STATUS_INTERNAL_ERROR;
 				}
 
 				/* Use the client requested access mask here,
 				 * not the one we open with. */
-				fsp_dup = fcb_or_dos_open(conn, fname, id,
+				status = fcb_or_dos_open(conn,
+							  fsp,
+							  fname,
+							  id,
 							  req->smbpid,
 							  req->vuid,
 							  access_mask,
 							  share_access,
 							  create_options);
 
-				if (fsp_dup) {
+				if (NT_STATUS_IS_OK(status)) {
 					TALLOC_FREE(lck);
-					file_free(fsp);
 					if (pinfo) {
 						*pinfo = FILE_WAS_OPENED;
 					}
-					conn->num_files_open++;
-					*result = fsp_dup;
 					return NT_STATUS_OK;
 				}
 			}
@@ -1628,7 +1613,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			} else {
 				status = NT_STATUS_ACCESS_DENIED;
 			}
-			file_free(fsp);
 			return status;
 		}
 
@@ -1666,7 +1650,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		if (lck != NULL) {
 			TALLOC_FREE(lck);
 		}
-		file_free(fsp);
 		return fsp_open;
 	}
 
@@ -1697,7 +1680,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			DEBUG(0, ("open_file_ntcreate: Could not get share "
 				  "mode lock for %s\n", fname));
 			fd_close(fsp);
-			file_free(fsp);
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 
@@ -1708,7 +1690,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-			file_free(fsp);
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 
@@ -1727,7 +1708,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 				schedule_defer_open(lck, request_time, req);
 				TALLOC_FREE(lck);
 				fd_close(fsp);
-				file_free(fsp);
 				return NT_STATUS_SHARING_VIOLATION;
 			}
 		}
@@ -1736,7 +1716,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			struct deferred_open_record state;
 
 			fd_close(fsp);
-			file_free(fsp);
 
 			state.delayed_for_oplocks = False;
 			state.id = id;
@@ -1778,7 +1757,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-			file_free(fsp);
 
 			return NT_STATUS_SHARING_VIOLATION;
 		}
@@ -1804,7 +1782,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			status = map_nt_error_from_unix(errno);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-			file_free(fsp);
 			return status;
 		}
 	}
@@ -1864,7 +1841,6 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			del_share_mode(lck, fsp);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-			file_free(fsp);
 			return status;
 		}
 		/* Note that here we set the *inital* delete on close flag,
@@ -1940,10 +1916,57 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 	}
 	TALLOC_FREE(lck);
 
-	conn->num_files_open++;
+	return NT_STATUS_OK;
+}
+
+/****************************************************************************
+ Open a file with a share mode.
+****************************************************************************/
+
+NTSTATUS open_file_ntcreate(connection_struct *conn,
+			    struct smb_request *req,
+			    const char *fname,
+			    SMB_STRUCT_STAT *psbuf,
+			    uint32 access_mask,		/* access bits (FILE_READ_DATA etc.) */
+			    uint32 share_access,	/* share constants (FILE_SHARE_READ etc) */
+			    uint32 create_disposition,	/* FILE_OPEN_IF etc. */
+			    uint32 create_options,	/* options such as delete on close. */
+			    uint32 new_dos_attributes,	/* attributes used for new file. */
+			    int oplock_request, 	/* internal Samba oplock codes. */
+				 			/* Information (FILE_EXISTS etc.) */
+			    int *pinfo,
+			    files_struct **result)
+{
+	NTSTATUS status;
+	files_struct *fsp = NULL;
+
+	*result = NULL;
+
+	status = file_new(conn, &fsp);
+	if(!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	status = open_file_ntcreate_internal(conn,
+					req,
+					fname,
+					psbuf,
+					access_mask,
+					share_access,
+					create_disposition,
+					create_options,
+					new_dos_attributes,
+					oplock_request,
+					pinfo,
+					fsp);
+
+	if(!NT_STATUS_IS_OK(status)) {
+		file_free(fsp);
+		return status;
+	}
 
 	*result = fsp;
-	return NT_STATUS_OK;
+	return status;
 }
 
 /****************************************************************************
@@ -1970,10 +1993,9 @@ NTSTATUS open_file_fchmod(connection_struct *conn, const char *fname,
 	status = open_file(fsp, conn, NULL, NULL, NULL, fname, psbuf, O_WRONLY,
 			   0, FILE_WRITE_DATA, FILE_WRITE_DATA);
 
-	/* 
+	/*
 	 * This is not a user visible file open.
-	 * Don't set a share mode and don't increment
-	 * the conn->num_files_open.
+	 * Don't set a share mode.
 	 */
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -2278,8 +2300,6 @@ NTSTATUS open_directory(connection_struct *conn,
 		*pinfo = info;
 	}
 
-	conn->num_files_open++;
-
 	*result = fsp;
 	return NT_STATUS_OK;
 }
@@ -2354,8 +2374,6 @@ NTSTATUS open_file_stat(connection_struct *conn, struct smb_request *req,
 	fsp->is_stat = True;
 	string_set(&fsp->fsp_name,fname);
 
-	conn->num_files_open++;
-
 	*result = fsp;
 	return NT_STATUS_OK;
 }
@@ -3050,6 +3068,7 @@ NTSTATUS create_file(connection_struct *conn,
 			 * also tries a QUERY_FILE_INFO on the file and then
 			 * close it
 			 */
+
 			status = open_fake_file(conn, fake_file_type, fname,
 						access_mask, &fsp);
 			if (!NT_STATUS_IS_OK(status)) {
diff --git a/source/smbd/reply.c b/source/smbd/reply.c
index 7aeefea98ba..f78e8bd0835 100644
--- a/source/smbd/reply.c
+++ b/source/smbd/reply.c
@@ -4614,10 +4614,17 @@ void reply_printopen(struct smb_request *req)
 		return;
 	}
 
+	status = file_new(conn, &fsp);
+	if(!NT_STATUS_IS_OK(status)) {
+		END_PROFILE(SMBsplopen);
+		return;
+	}
+
 	/* Open for exclusive use, write only. */
-	status = print_fsp_open(conn, NULL, &fsp);
+	status = print_fsp_open(conn, NULL, fsp);
 
 	if (!NT_STATUS_IS_OK(status)) {
+		file_free(fsp);
 		reply_nterror(req, status);
 		END_PROFILE(SMBsplopen);
 		return;