Attachment 15212 Details for Bug 13979 – git am for master - prototype fix for mkdir

[patch] git am for master - prototype fix for mkdir

0001-WIP-prototype-fix-for-mkdir-races.patch (text/plain), 7.49 KB, created by Jeremy Allison on 2019-05-31 22:38:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2019-05-31 22:38:55 UTC

Size: 7.49 KB

patch

obsolete

>From 006eee8e923dac2b34ead6d6277907668ab6457c Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 31 May 2019 14:46:16 -0700
>Subject: [PATCH] WIP: prototype fix for mkdir races.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 182 ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 160 insertions(+), 22 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 7a0db9396f9..d2b03c430b0 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -3895,6 +3895,113 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 	return NT_STATUS_OK;
> }
> 
>+struct safe_pathname_state {
>+	connection_struct *conn;
>+	struct smb_filename *old_cwd;
>+};
>+
>+static NTSTATUS safe_pathname_start(TALLOC_CTX *mem_ctx,
>+			connection_struct *conn,
>+			const struct smb_filename *smb_fname_in,
>+			struct smb_filename **pp_smb_fname_out,
>+			struct safe_pathname_state **pp_safe_pathname_state)
>+{
>+	struct safe_pathname_state *state = NULL;
>+	const char *final_component = NULL;
>+	char *parent_dir = NULL;
>+	bool ok = false;
>+	bool done_chdir = false;
>+	struct smb_filename parent_dir_fname = {0};
>+	struct smb_filename *smb_fname_rel = NULL;
>+	int ret = -1;
>+	NTSTATUS status;
>+
>+	/* This can *never* be done on a stream name. */
>+	if (smb_fname_in->stream_name != NULL) {
>+		return NT_STATUS_INVALID_PARAMETER;
>+	}
>+
>+	state = talloc_zero(mem_ctx, struct safe_pathname_state);
>+	if (state == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+
>+	state->conn = conn;
>+
>+	ok = parent_dirname(state,
>+			    smb_fname_in->base_name,
>+			    &parent_dir,
>+			    &final_component);
>+	if (!ok) {
>+		status = NT_STATUS_NO_MEMORY;
>+		goto err;
>+	}
>+
>+	state->old_cwd = vfs_GetWd(state, conn);
>+	if (state->old_cwd == NULL) {
>+		status = map_nt_error_from_unix(errno);
>+		goto err;
>+	}
>+
>+	parent_dir_fname = (struct smb_filename) { .base_name = parent_dir };
>+
>+	/* Pin parent directory in place. */
>+	ret = vfs_ChDir(conn, &parent_dir_fname);
>+	if (ret == -1) {
>+		status = map_nt_error_from_unix(errno);
>+		goto err;
>+	}
>+
>+	done_chdir = true;
>+
>+	smb_fname_rel = synthetic_smb_fname(state,
>+				final_component,
>+				NULL,
>+				&smb_fname_in->st,
>+				smb_fname_in->flags);
>+	if (smb_fname_rel == NULL) {
>+		status = NT_STATUS_NO_MEMORY;
>+		goto err;
>+        }
>+
>+	/* Ensure the relative path is below the share. */
>+	status = check_reduced_name(conn, &parent_dir_fname, smb_fname_rel);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		goto err;
>+	}
>+
>+	/*
>+	 * We are safe. Exit with the new $cwd.
>+	 * safe_pathname_end() will undo this.
>+	 */
>+
>+	*pp_safe_pathname_state = state;
>+	*pp_smb_fname_out = smb_fname_rel;
>+	return NT_STATUS_OK;
>+
>+  err:
>+
>+	if (done_chdir) {
>+		ret = vfs_ChDir(conn, state->old_cwd);
>+		if (ret == -1) {
>+			smb_panic("unable to get back to old directory\n");
>+		}
>+	}
>+	TALLOC_FREE(state);
>+	return status;
>+}
>+
>+static void safe_pathname_end(struct safe_pathname_state **pp_state)
>+{
>+	int ret = -1;
>+	/* Chdir back */
>+	ret = vfs_ChDir((*pp_state)->conn, (*pp_state)->old_cwd);
>+	if (ret == -1) {
>+		smb_panic("unable to get back to old directory\n");
>+	}
>+	TALLOC_FREE(*pp_state);
>+}
>+
> static NTSTATUS mkdir_internal(connection_struct *conn,
> 			       struct smb_filename *smb_dname,
> 			       uint32_t file_attributes)
>@@ -3905,6 +4012,8 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
> 	bool posix_open = false;
> 	bool need_re_stat = false;
> 	uint32_t access_mask = SEC_DIR_ADD_SUBDIR;
>+	struct smb_filename *smb_dname_rel = NULL;
>+	struct safe_pathname_state *safe_filename_state = NULL;
> 
> 	if (!CAN_WRITE(conn) || (access_mask & ~(conn->share_access))) {
> 		DEBUG(5,("mkdir_internal: failing share access "
>@@ -3924,48 +4033,65 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
> 		mode = unix_mode(conn, FILE_ATTRIBUTE_DIRECTORY, smb_dname, parent_dir);
> 	}
> 
>-	status = check_parent_access(conn,
>+	status = safe_pathname_start(talloc_tos(),
>+					conn,
> 					smb_dname,
>+					&smb_dname_rel,
>+					&safe_filename_state);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DEBUG(5,("safe_pathname_start "
>+			"on directory %s for path %s returned %s\n",
>+			parent_dir,
>+			smb_dname->base_name,
>+			nt_errstr(status) ));
>+		return status;
>+	}
>+
>+	status = check_parent_access(conn,
>+					smb_dname_rel,
> 					access_mask);
> 	if(!NT_STATUS_IS_OK(status)) {
>-		DEBUG(5,("mkdir_internal: check_parent_access "
>+		DEBUG(5,("check_parent_access "
> 			"on directory %s for path %s returned %s\n",
> 			parent_dir,
> 			smb_dname->base_name,
> 			nt_errstr(status) ));
>-		return status;
>+		goto err;
> 	}
> 
>-	if (SMB_VFS_MKDIR(conn, smb_dname, mode) != 0) {
>-		return map_nt_error_from_unix(errno);
>+	if (SMB_VFS_MKDIR(conn, smb_dname_rel, mode) != 0) {
>+		status = map_nt_error_from_unix(errno);
>+		goto err;
> 	}
> 
> 	/* Ensure we're checking for a symlink here.... */
> 	/* We don't want to get caught by a symlink racer. */
> 
>-	if (SMB_VFS_LSTAT(conn, smb_dname) == -1) {
>+	if (SMB_VFS_LSTAT(conn, smb_dname_rel) == -1) {
> 		DEBUG(2, ("Could not stat directory '%s' just created: %s\n",
> 			  smb_fname_str_dbg(smb_dname), strerror(errno)));
>-		return map_nt_error_from_unix(errno);
>+		status = map_nt_error_from_unix(errno);
>+		goto err;
> 	}
> 
>-	if (!S_ISDIR(smb_dname->st.st_ex_mode)) {
>+	if (!S_ISDIR(smb_dname_rel->st.st_ex_mode)) {
> 		DEBUG(0, ("Directory '%s' just created is not a directory !\n",
> 			  smb_fname_str_dbg(smb_dname)));
>-		return NT_STATUS_NOT_A_DIRECTORY;
>+		status = NT_STATUS_NOT_A_DIRECTORY;
>+		goto err;
> 	}
> 
> 	if (lp_store_dos_attributes(SNUM(conn))) {
> 		if (!posix_open) {
>-			file_set_dosmode(conn, smb_dname,
>+			file_set_dosmode(conn, smb_dname_rel,
> 					 file_attributes | FILE_ATTRIBUTE_DIRECTORY,
>-					 parent_dir, true);
>+					 ".", true);
> 		}
> 	}
> 
> 	if (lp_inherit_permissions(SNUM(conn))) {
>-		inherit_access_posix_acl(conn, parent_dir,
>-					 smb_dname, mode);
>+		inherit_access_posix_acl(conn, ".",
>+					 smb_dname_rel, mode);
> 		need_re_stat = true;
> 	}
> 
>@@ -3977,34 +4103,46 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
> 		 * dir.
> 		 */
> 		if ((mode & ~(S_IRWXU|S_IRWXG|S_IRWXO)) &&
>-		    (mode & ~smb_dname->st.st_ex_mode)) {
>-			SMB_VFS_CHMOD(conn, smb_dname,
>-				      (smb_dname->st.st_ex_mode |
>-					  (mode & ~smb_dname->st.st_ex_mode)));
>+		    (mode & ~smb_dname_rel->st.st_ex_mode)) {
>+			SMB_VFS_CHMOD(conn, smb_dname_rel,
>+				      (smb_dname_rel->st.st_ex_mode |
>+					  (mode & ~smb_dname_rel->st.st_ex_mode)));
> 			need_re_stat = true;
> 		}
> 	}
> 
> 	/* Change the owner if required. */
> 	if (lp_inherit_owner(SNUM(conn)) != INHERIT_OWNER_NO) {
>-		change_dir_owner_to_parent(conn, parent_dir,
>-					   smb_dname,
>-					   &smb_dname->st);
>+		change_dir_owner_to_parent(conn, ".",
>+					   smb_dname_rel,
>+					   &smb_dname_rel->st);
> 		need_re_stat = true;
> 	}
> 
> 	if (need_re_stat) {
>-		if (SMB_VFS_LSTAT(conn, smb_dname) == -1) {
>+		if (SMB_VFS_LSTAT(conn, smb_dname_rel) == -1) {
>+			status = map_nt_error_from_unix(errno);
> 			DEBUG(2, ("Could not stat directory '%s' just created: %s\n",
> 			  smb_fname_str_dbg(smb_dname), strerror(errno)));
>-			return map_nt_error_from_unix(errno);
>+			goto err;
> 		}
> 	}
> 
>+	/* Go back to the previous $cwd. */
>+	safe_pathname_end(&safe_filename_state);
>+
>+	/* Ensure returned stat is up to date. */
>+	smb_dname->st = smb_dname_rel->st;
>+
> 	notify_fname(conn, NOTIFY_ACTION_ADDED, FILE_NOTIFY_CHANGE_DIR_NAME,
> 		     smb_dname->base_name);
> 
> 	return NT_STATUS_OK;
>+
>+  err:
>+
>+	safe_pathname_end(&safe_filename_state);
>+	return status;
> }
> 
> /****************************************************************************
>-- 
>2.22.0.rc1.257.g3120a18244-goog
>

Actions: View

Attachments on bug 13979: 15205 | 15212 | 15213 | 15214 | 15215 | 15217 | 16812 | 16815 | 16987 | 16988 | 17072