smbd: NULL out "fsp" in close_file()

Quite a few places already had this in the caller, but not all. Rename close_file() to close_file_free() appropriately. We'll factor out close_file_smb() doing only parts of close_file_free() later. Signed-off-by: Volker Lendecke <[email protected]> Reviewed-by: Jeremy Allison <[email protected]>
remicollet · Feb 10, 2022 · f5bc73a · f5bc73a
1 parent 363ac75
commit f5bc73a
Show file tree

Hide file tree

Showing 16 changed files with 103 additions and 116 deletions.
diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
@@ -1254,13 +1254,13 @@ static bool ad_convert_xattr(vfs_handle_struct *handle,
 		if (nwritten == -1) {
 			DBG_ERR("SMB_VFS_PWRITE failed\n");
 			saved_errno = errno;
-			close_file(NULL, fsp, ERROR_CLOSE);
+			close_file_free(NULL, &fsp, ERROR_CLOSE);
 			errno = saved_errno;
 			ok = false;
 			goto fail;
 		}
 
-		status = close_file(NULL, fsp, NORMAL_CLOSE);
+		status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
 			ok = false;
 			goto fail;
@@ -1395,12 +1395,12 @@ static bool ad_convert_finderinfo(vfs_handle_struct *handle,
 	if (nwritten == -1) {
 		DBG_ERR("SMB_VFS_PWRITE failed\n");
 		saved_errno = errno;
-		close_file(NULL, fsp, ERROR_CLOSE);
+		close_file_free(NULL, &fsp, ERROR_CLOSE);
 		errno = saved_errno;
 		return false;
 	}
 
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
+	status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
@@ -1652,7 +1652,7 @@ static bool ad_unconvert_open_ad(TALLOC_CTX *mem_ctx,
 		if (ret != 0) {
 			DBG_ERR("SMB_VFS_FCHOWN [%s] failed: %s\n",
 				fsp_str_dbg(fsp), nt_errstr(status));
-			close_file(NULL, fsp, NORMAL_CLOSE);
+			close_file_free(NULL, &fsp, NORMAL_CLOSE);
 			return false;
 		}
 	}
@@ -1710,14 +1710,14 @@ static bool ad_unconvert_get_streams(struct vfs_handle_struct *handle,
 				num_streams,
 				streams);
 	if (!NT_STATUS_IS_OK(status)) {
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		DBG_ERR("streaminfo on [%s] failed: %s\n",
 			smb_fname_str_dbg(smb_fname),
 			nt_errstr(status));
 		return false;
 	}
 
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
+	status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_ERR("close_file [%s] failed: %s\n",
 			smb_fname_str_dbg(smb_fname),
@@ -1975,7 +1975,7 @@ static bool ad_collect_one_stream(struct vfs_handle_struct *handle,
 out:
 	TALLOC_FREE(sname);
 	if (fsp != NULL) {
-		status = close_file(NULL, fsp, NORMAL_CLOSE);
+		status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("close_file [%s] failed: %s\n",
 				smb_fname_str_dbg(smb_fname),
@@ -2117,9 +2117,9 @@ bool ad_unconvert(TALLOC_CTX *mem_ctx,
 
 out:
 	if (fsp != NULL) {
-		status = close_file(NULL, fsp, NORMAL_CLOSE);
+		status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			DBG_ERR("close_file [%s] failed: %s\n",
+			DBG_ERR("close_file_free() [%s] failed: %s\n",
 				smb_fname_str_dbg(smb_fname),
 				nt_errstr(status));
 			ok = false;

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
@@ -1002,7 +1002,7 @@ static bool readdir_attr_meta_finderi_stream(
 
 fail:
 	if (fsp != NULL) {
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	}
 
 	return ok;
@@ -4247,8 +4247,8 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
 	DEBUG(10, ("fruit_create_file: %s\n", nt_errstr(status)));
 
 	if (fsp) {
-		close_file(req, fsp, ERROR_CLOSE);
-		*result = fsp = NULL;
+		close_file_free(req, &fsp, ERROR_CLOSE);
+		*result = NULL;
 	}
 
 	return status;
@@ -4993,8 +4993,7 @@ static bool fruit_get_bandsize(vfs_handle_struct *handle,
 
 	}
 
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
-	fsp = NULL;
+	status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_ERR("close_file failed: %s\n", nt_errstr(status));
 		ok = false;
@@ -5028,11 +5027,10 @@ static bool fruit_get_bandsize(vfs_handle_struct *handle,
 
 out:
 	if (fsp != NULL) {
-		status = close_file(NULL, fsp, NORMAL_CLOSE);
+		status = close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("close_file failed: %s\n", nt_errstr(status));
 		}
-		fsp = NULL;
 	}
 	TALLOC_FREE(plist);
 	TALLOC_FREE(smb_fname);

diff --git a/source3/modules/vfs_worm.c b/source3/modules/vfs_worm.c
@@ -75,7 +75,7 @@ static NTSTATUS vfs_worm_create_file(vfs_handle_struct *handle,
 	 * Access via MAXIMUM_ALLOWED_ACCESS?
 	 */
 	if (readonly && ((*result)->access_mask & write_access_flags)) {
-		close_file(req, *result, NORMAL_CLOSE);
+		close_file_free(req, result, NORMAL_CLOSE);
 		return NT_STATUS_ACCESS_DENIED;
 	}
 	return NT_STATUS_OK;

diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
@@ -874,8 +874,7 @@ static int file_version_is_newer(connection_struct *conn, fstring new_file, fstr
 			    (long)old_create_time));
 	}
 
-	close_file(NULL, fsp, NORMAL_CLOSE);
-	fsp = NULL;
+	close_file_free(NULL, &fsp, NORMAL_CLOSE);
 
 	/* Get file version info (if available) for new file */
 	status = driver_unix_convert(conn, new_file, &smb_fname);
@@ -935,8 +934,7 @@ static int file_version_is_newer(connection_struct *conn, fstring new_file, fstr
 			    (long)new_create_time));
 	}
 
-	close_file(NULL, fsp, NORMAL_CLOSE);
-	fsp = NULL;
+	close_file_free(NULL, &fsp, NORMAL_CLOSE);
 
 	if (use_version && (new_major != old_major || new_minor != old_minor)) {
 		/* Compare versions and choose the larger version number */
@@ -969,7 +967,7 @@ static int file_version_is_newer(connection_struct *conn, fstring new_file, fstr
 
  error_exit:
 	if(fsp)
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	ret = -1;
  done:
 	TALLOC_FREE(smb_fname);
@@ -1177,7 +1175,7 @@ static uint32_t get_correct_cversion(const struct auth_session_info *session_inf
 	unbecome_user_without_service();
  error_free_conn:
 	if (fsp != NULL) {
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	}
 	if (!W_ERROR_IS_OK(*perr)) {
 		cversion = -1;

diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -2539,7 +2539,7 @@ WERROR _srvsvc_NetGetFileSecurity(struct pipes_struct *p,
 error_exit:
 
 	if (fsp) {
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	}
 
 	TALLOC_FREE(frame);
@@ -2659,7 +2659,7 @@ WERROR _srvsvc_NetSetFileSecurity(struct pipes_struct *p,
 error_exit:
 
 	if (fsp) {
-		close_file(NULL, fsp, NORMAL_CLOSE);
+		close_file_free(NULL, &fsp, NORMAL_CLOSE);
 	}
 
 	TALLOC_FREE(frame);

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
@@ -1478,10 +1478,12 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp,
  Close a files_struct.
 ****************************************************************************/
 
-NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
-		    enum file_close_type close_type)
+NTSTATUS close_file_free(struct smb_request *req,
+			 struct files_struct **_fsp,
+			 enum file_close_type close_type)
 {
 	NTSTATUS status;
+	struct files_struct *fsp = *_fsp;
 	struct files_struct *base_fsp = fsp->base_fsp;
 	bool close_base_fsp = false;
 
@@ -1569,9 +1571,11 @@ NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
 		 * those loops will become confused.
 		 */
 
-		close_file(req, base_fsp, close_type);
+		close_file_free(req, &base_fsp, close_type);
 	}
 
+	*_fsp = NULL;
+
 	return status;
 }
 
@@ -1610,5 +1614,5 @@ void msg_close_file(struct messaging_context *msg_ctx,
 		DEBUG(10,("msg_close_file: failed to find file.\n"));
 		return;
 	}
-	close_file(NULL, fsp, NORMAL_CLOSE);
+	close_file_free(NULL, &fsp, NORMAL_CLOSE);
 }
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
@@ -185,9 +185,12 @@ void dptr_closecnum(connection_struct *conn)
 	for(dptr = sconn->searches.dirptrs; dptr; dptr = next) {
 		next = dptr->next;
 		if (dptr->conn == conn) {
-			files_struct *fsp = dptr->dir_hnd->fsp;
-			close_file(NULL, fsp, NORMAL_CLOSE);
-			fsp = NULL;
+			/*
+			 * Need to make a copy, "dptr" will be gone
+			 * after close_file_free() returns
+			 */
+			struct files_struct *fsp = dptr->dir_hnd->fsp;
+			close_file_free(NULL, &fsp, NORMAL_CLOSE);
 		}
 	}
 }

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
@@ -778,7 +778,7 @@ void file_close_conn(connection_struct *conn)
 			 */
 			fsp->op->global->durable = false;
 		}
-		close_file(NULL, fsp, SHUTDOWN_CLOSE);
+		close_file_free(NULL, &fsp, SHUTDOWN_CLOSE);
 	}
 }
 
@@ -848,7 +848,7 @@ void file_close_user(struct smbd_server_connection *sconn, uint64_t vuid)
 	for (fsp=sconn->files; fsp; fsp=next) {
 		next=fsp->next;
 		if (fsp->vuid == vuid) {
-			close_file(NULL, fsp, SHUTDOWN_CLOSE);
+			close_file_free(NULL, &fsp, SHUTDOWN_CLOSE);
 		}
 	}
 }

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
@@ -1649,7 +1649,7 @@ NTSTATUS copy_internals(TALLOC_CTX *ctx,
 		NULL, NULL);				/* create context */
 
 	if (!NT_STATUS_IS_OK(status)) {
-		close_file(NULL, fsp1, ERROR_CLOSE);
+		close_file_free(NULL, &fsp1, ERROR_CLOSE);
 		goto out;
 	}
 
@@ -1663,12 +1663,12 @@ NTSTATUS copy_internals(TALLOC_CTX *ctx,
 	 * Thus we don't look at the error return from the
 	 * close of fsp1.
 	 */
-	close_file(NULL, fsp1, NORMAL_CLOSE);
+	close_file_free(NULL, &fsp1, NORMAL_CLOSE);
 
 	/* Ensure the modtime is set correctly on the destination file. */
 	set_close_write_time(fsp2, smb_fname_src->st.st_ex_mtime);
 
-	status = close_file(NULL, fsp2, NORMAL_CLOSE);
+	status = close_file_free(NULL, &fsp2, NORMAL_CLOSE);
 
 	/* Grrr. We have to do this as open_file_ntcreate adds FILE_ATTRIBUTE_ARCHIVE when it
 	   creates the file. This isn't the correct thing to do in the copy

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
@@ -4765,7 +4765,7 @@ NTSTATUS create_directory(connection_struct *conn, struct smb_request *req,
 		NULL, NULL);				/* create context */
 
 	if (NT_STATUS_IS_OK(status)) {
-		close_file(req, fsp, NORMAL_CLOSE);
+		close_file_free(req, &fsp, NORMAL_CLOSE);
 	}
 
 	return status;
@@ -5015,7 +5015,7 @@ static NTSTATUS open_streams_for_delete(connection_struct *conn,
 
 		DEBUG(10, ("Closing stream # %d, %s\n", j,
 			   fsp_str_dbg(streams[j])));
-		close_file(NULL, streams[j], NORMAL_CLOSE);
+		close_file_free(NULL, &streams[j], NORMAL_CLOSE);
 	}
 
  fail:
@@ -6058,12 +6058,10 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
 		 * fsp->base_fsp.
 		 */
 		base_fsp = NULL;
-		close_file(req, fsp, ERROR_CLOSE);
-		fsp = NULL;
+		close_file_free(req, &fsp, ERROR_CLOSE);
 	}
 	if (base_fsp != NULL) {
-		close_file(req, base_fsp, ERROR_CLOSE);
-		base_fsp = NULL;
+		close_file_free(req, &base_fsp, ERROR_CLOSE);
 	}
 
 	TALLOC_FREE(parent_dir_fname);
@@ -6241,8 +6239,7 @@ NTSTATUS create_file_default(connection_struct *conn,
 	DEBUG(10, ("create_file: %s\n", nt_errstr(status)));
 
 	if (fsp != NULL) {
-		close_file(req, fsp, ERROR_CLOSE);
-		fsp = NULL;
+		close_file_free(req, &fsp, ERROR_CLOSE);
 	}
 	return status;
 }
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
@@ -130,8 +130,9 @@ bool smbd_smb1_brl_finish_by_mid(
 /* The following definitions come from smbd/close.c  */
 
 void set_close_write_time(struct files_struct *fsp, struct timespec ts);
-NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
-		    enum file_close_type close_type);
+NTSTATUS close_file_free(struct smb_request *req,
+			 struct files_struct **_fsp,
+			 enum file_close_type close_type);
 void msg_close_file(struct messaging_context *msg_ctx,
 		    void *private_data,
 		    uint32_t msg_type,