From: Ralph Boehme <slow@samba.org>
Date: Thu, 23 Apr 2020 09:46:19 +0000 (+0200)
Subject: smbd: use an early exit if stat succeeds in unix_convert_step_stat()
X-Git-Tag: ldb-2.2.0~877
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5b0271e0c3d4354f0e7dfc3eb149607e7cc118f;p=thirdparty%2Fsamba.git

smbd: use an early exit if stat succeeds in unix_convert_step_stat()

Allows decreasing the indentation level of the bulk of the code that handles
stat failure. Best viewed with `git show -w`.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index ed6a5c95c41..75916c88d2a 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -478,6 +478,7 @@ struct uc_state {
 
 static NTSTATUS unix_convert_step_stat(struct uc_state *state)
 {
+	char *found_name = NULL;
 	int ret;
 
 	/*
@@ -511,241 +512,182 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state)
 			 */
 			return NT_STATUS_OBJECT_PATH_NOT_FOUND;
 		}
+		return NT_STATUS_OK;
+	}
 
-	} else {
-		char *found_name = NULL;
+	/* Stat failed - ensure we don't use it. */
+	SET_STAT_INVALID(state->smb_fname->st);
 
-		/* Stat failed - ensure we don't use it. */
-		SET_STAT_INVALID(state->smb_fname->st);
-
-		if (state->posix_pathnames) {
+	if (state->posix_pathnames) {
+		/*
+		 * For posix_pathnames, we're done.
+		 * Don't blunder into the name_has_wildcard OR
+		 * get_real_filename() codepaths as they may
+		 * be doing case insensitive lookups. So when
+		 * creating a new POSIX directory Foo they might
+		 * match on name foo.
+		 *
+		 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13803
+		 */
+		if (state->end != NULL) {
+			const char *morepath = NULL;
 			/*
-			 * For posix_pathnames, we're done.
-			 * Don't blunder into the name_has_wildcard OR
-			 * get_real_filename() codepaths as they may
-			 * be doing case insensitive lookups. So when
-			 * creating a new POSIX directory Foo they might
-			 * match on name foo.
-			 *
-			 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13803
+			 * If this is intermediate we must
+			 * restore the full path.
 			 */
-			if (state->end != NULL) {
-				const char *morepath = NULL;
-				/*
-				 * If this is intermediate we must
-				 * restore the full path.
-				 */
-				*state->end = '/';
-				/*
-				 * If there are any more components
-				 * after the failed LSTAT we cannot
-				 * continue.
-				 */
-				morepath = strchr(state->end + 1, '/');
-				if (morepath != NULL) {
-					return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-				}
-			}
-			if (errno == ENOENT) {
-				/* New file or directory. */
-				state->done = true;
-				return NT_STATUS_OK;
-			}
-			if ((errno == EACCES) &&
-			    (state->ucf_flags & UCF_PREP_CREATEFILE)) {
-				/* POSIX Dropbox case. */
-				errno = 0;
-				state->done = true;
-				return NT_STATUS_OK;
+			*state->end = '/';
+			/*
+			 * If there are any more components
+			 * after the failed LSTAT we cannot
+			 * continue.
+			 */
+			morepath = strchr(state->end + 1, '/');
+			if (morepath != NULL) {
+				return NT_STATUS_OBJECT_PATH_NOT_FOUND;
 			}
-			return map_nt_error_from_unix(errno);
 		}
+		if (errno == ENOENT) {
+			/* New file or directory. */
+			state->done = true;
+			return NT_STATUS_OK;
+		}
+		if ((errno == EACCES) &&
+		    (state->ucf_flags & UCF_PREP_CREATEFILE)) {
+			/* POSIX Dropbox case. */
+			errno = 0;
+			state->done = true;
+			return NT_STATUS_OK;
+		}
+		return map_nt_error_from_unix(errno);
+	}
 
-		/*
-		 * Reset errno so we can detect
-		 * directory open errors.
-		 */
-		errno = 0;
-
-		/*
-		 * Try to find this part of the path in the directory.
-		 */
-
-		if (state->name_has_wildcard ||
-		    (get_real_filename(state->conn, state->dirpath, state->name,
-				       talloc_tos(),
-				       &found_name) == -1)) {
-			char *unmangled;
+	/*
+	 * Reset errno so we can detect
+	 * directory open errors.
+	 */
+	errno = 0;
 
-			if (state->end) {
-				/*
-				 * An intermediate part of the name
-				 * can't be found.
-				 */
-				DBG_DEBUG("Intermediate [%s] missing\n",
-					  state->name);
-				*state->end = '/';
+	/*
+	 * Try to find this part of the path in the directory.
+	 */
 
-				/*
-				 * We need to return the fact that the
-				 * intermediate name resolution failed.
-				 * This is used to return an error of
-				 * ERRbadpath rather than ERRbadfile.
-				 * Some Windows applications depend on
-				 * the difference between these two
-				 * errors.
-				 */
+	if (state->name_has_wildcard ||
+	    (get_real_filename(state->conn, state->dirpath, state->name,
+			       talloc_tos(),
+			       &found_name) == -1)) {
+		char *unmangled;
 
-				/*
-				 * ENOENT, ENOTDIR and ELOOP all map
-				 * to NT_STATUS_OBJECT_PATH_NOT_FOUND
-				 * in the filename walk.
-				 */
+		if (state->end) {
+			/*
+			 * An intermediate part of the name
+			 * can't be found.
+			 */
+			DBG_DEBUG("Intermediate [%s] missing\n",
+				  state->name);
+			*state->end = '/';
 
-				if (errno == ENOENT ||
-				    errno == ENOTDIR ||
-				    errno == ELOOP)
-				{
-					return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-				}
-				return map_nt_error_from_unix(errno);
-			}
+			/*
+			 * We need to return the fact that the
+			 * intermediate name resolution failed.
+			 * This is used to return an error of
+			 * ERRbadpath rather than ERRbadfile.
+			 * Some Windows applications depend on
+			 * the difference between these two
+			 * errors.
+			 */
 
 			/*
-			 * ENOENT/EACCESS are the only valid errors
-			 * here.
+			 * ENOENT, ENOTDIR and ELOOP all map
+			 * to NT_STATUS_OBJECT_PATH_NOT_FOUND
+			 * in the filename walk.
 			 */
 
-			if (errno == EACCES) {
-				if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) {
-					return NT_STATUS_ACCESS_DENIED;
-				} else {
-					/*
-					 * This is the dropbox
-					 * behaviour. A dropbox is a
-					 * directory with only -wx
-					 * permissions, so
-					 * get_real_filename fails
-					 * with EACCESS, it needs to
-					 * list the directory. We
-					 * nevertheless want to allow
-					 * users creating a file.
-					 */
-					errno = 0;
-				}
+			if (errno == ENOENT ||
+			    errno == ENOTDIR ||
+			    errno == ELOOP)
+			{
+				return NT_STATUS_OBJECT_PATH_NOT_FOUND;
 			}
+			return map_nt_error_from_unix(errno);
+		}
+
+		/*
+		 * ENOENT/EACCESS are the only valid errors
+		 * here.
+		 */
 
-			if ((errno != 0) && (errno != ENOENT)) {
+		if (errno == EACCES) {
+			if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) {
+				return NT_STATUS_ACCESS_DENIED;
+			} else {
 				/*
-				 * ENOTDIR and ELOOP both map to
-				 * NT_STATUS_OBJECT_PATH_NOT_FOUND
-				 * in the filename walk.
+				 * This is the dropbox
+				 * behaviour. A dropbox is a
+				 * directory with only -wx
+				 * permissions, so
+				 * get_real_filename fails
+				 * with EACCESS, it needs to
+				 * list the directory. We
+				 * nevertheless want to allow
+				 * users creating a file.
 				 */
-				if (errno == ENOTDIR || errno == ELOOP) {
-					return NT_STATUS_OBJECT_PATH_NOT_FOUND;
-				}
-				return map_nt_error_from_unix(errno);
+				errno = 0;
 			}
+		}
 
+		if ((errno != 0) && (errno != ENOENT)) {
 			/*
-			 * Just the last part of the name doesn't exist.
-			 * We need to strupper() or strlower() it as
-			 * this conversion may be used for file creation
-			 * purposes. Fix inspired by
-			 * Thomas Neumann <t.neumann@iku-ag.de>.
+			 * ENOTDIR and ELOOP both map to
+			 * NT_STATUS_OBJECT_PATH_NOT_FOUND
+			 * in the filename walk.
 			 */
-			if (!state->conn->case_preserve ||
-			    (mangle_is_8_3(state->name, false,
-					   state->conn->params) &&
-			     !state->conn->short_case_preserve)) {
-				if (!strnorm(state->name,
-					     lp_default_case(SNUM(state->conn)))) {
-					DBG_DEBUG("strnorm %s failed\n",
-						  state->name);
-					return NT_STATUS_INVALID_PARAMETER;
-				}
+			if (errno == ENOTDIR || errno == ELOOP) {
+				return NT_STATUS_OBJECT_PATH_NOT_FOUND;
 			}
+			return map_nt_error_from_unix(errno);
+		}
 
-			/*
-			 * check on the mangled stack to see if we can
-			 * recover the base of the filename.
-			 */
-
-			if (mangle_is_mangled(state->name, state->conn->params)
-			    && mangle_lookup_name_from_8_3(state->mem_ctx,
-							   state->name,
-							   &unmangled,
-							   state->conn->params)) {
-				char *tmp;
-				size_t name_ofs =
-					state->name - state->smb_fname->base_name;
-
-				if (!ISDOT(state->dirpath)) {
-					tmp = talloc_asprintf(
-						state->smb_fname, "%s/%s",
-						state->dirpath, unmangled);
-					TALLOC_FREE(unmangled);
-				}
-				else {
-					tmp = unmangled;
-				}
-				if (tmp == NULL) {
-					DBG_ERR("talloc failed\n");
-					return NT_STATUS_NO_MEMORY;
-				}
-				TALLOC_FREE(state->smb_fname->base_name);
-				state->smb_fname->base_name = tmp;
-				state->name =
-					state->smb_fname->base_name + name_ofs;
-				state->end = state->name + strlen(state->name);
+		/*
+		 * Just the last part of the name doesn't exist.
+		 * We need to strupper() or strlower() it as
+		 * this conversion may be used for file creation
+		 * purposes. Fix inspired by
+		 * Thomas Neumann <t.neumann@iku-ag.de>.
+		 */
+		if (!state->conn->case_preserve ||
+		    (mangle_is_8_3(state->name, false,
+				   state->conn->params) &&
+		     !state->conn->short_case_preserve)) {
+			if (!strnorm(state->name,
+				     lp_default_case(SNUM(state->conn)))) {
+				DBG_DEBUG("strnorm %s failed\n",
+					  state->name);
+				return NT_STATUS_INVALID_PARAMETER;
 			}
-
-			DBG_DEBUG("New file [%s]\n", state->name);
-			state->done = true;
-			return NT_STATUS_OK;
 		}
 
-
 		/*
-		 * Restore the rest of the string. If the string was
-		 * mangled the size may have changed.
+		 * check on the mangled stack to see if we can
+		 * recover the base of the filename.
 		 */
-		if (state->end) {
+
+		if (mangle_is_mangled(state->name, state->conn->params)
+		    && mangle_lookup_name_from_8_3(state->mem_ctx,
+						   state->name,
+						   &unmangled,
+						   state->conn->params)) {
 			char *tmp;
 			size_t name_ofs =
 				state->name - state->smb_fname->base_name;
 
 			if (!ISDOT(state->dirpath)) {
-				tmp = talloc_asprintf(state->smb_fname,
-						      "%s/%s/%s", state->dirpath,
-						      found_name, state->end+1);
+				tmp = talloc_asprintf(
+					state->smb_fname, "%s/%s",
+					state->dirpath, unmangled);
+				TALLOC_FREE(unmangled);
 			}
 			else {
-				tmp = talloc_asprintf(state->smb_fname,
-						      "%s/%s", found_name,
-						      state->end+1);
-			}
-			if (tmp == NULL) {
-				DBG_ERR("talloc_asprintf failed\n");
-				return NT_STATUS_NO_MEMORY;
-			}
-			TALLOC_FREE(state->smb_fname->base_name);
-			state->smb_fname->base_name = tmp;
-			state->name = state->smb_fname->base_name + name_ofs;
-			state->end = state->name + strlen(found_name);
-			*state->end = '\0';
-		} else {
-			char *tmp;
-			size_t name_ofs =
-				state->name - state->smb_fname->base_name;
-
-			if (!ISDOT(state->dirpath)) {
-				tmp = talloc_asprintf(state->smb_fname,
-						      "%s/%s", state->dirpath,
-						      found_name);
-			} else {
-				tmp = talloc_strdup(state->smb_fname,
-						    found_name);
+				tmp = unmangled;
 			}
 			if (tmp == NULL) {
 				DBG_ERR("talloc failed\n");
@@ -753,28 +695,84 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state)
 			}
 			TALLOC_FREE(state->smb_fname->base_name);
 			state->smb_fname->base_name = tmp;
-			state->name = state->smb_fname->base_name + name_ofs;
+			state->name =
+				state->smb_fname->base_name + name_ofs;
+			state->end = state->name + strlen(state->name);
+		}
 
-			/*
-			 * We just scanned for, and found the end of
-			 * the path. We must return a valid stat struct
-			 * if it exists. JRA.
-			 */
+		DBG_DEBUG("New file [%s]\n", state->name);
+		state->done = true;
+		return NT_STATUS_OK;
+	}
 
-			if (state->posix_pathnames) {
-				ret = SMB_VFS_LSTAT(state->conn, state->smb_fname);
-			} else {
-				ret = SMB_VFS_STAT(state->conn, state->smb_fname);
-			}
 
-			if (ret != 0) {
-				SET_STAT_INVALID(state->smb_fname->st);
-			}
+	/*
+	 * Restore the rest of the string. If the string was
+	 * mangled the size may have changed.
+	 */
+	if (state->end) {
+		char *tmp;
+		size_t name_ofs =
+			state->name - state->smb_fname->base_name;
+
+		if (!ISDOT(state->dirpath)) {
+			tmp = talloc_asprintf(state->smb_fname,
+					      "%s/%s/%s", state->dirpath,
+					      found_name, state->end+1);
+		}
+		else {
+			tmp = talloc_asprintf(state->smb_fname,
+					      "%s/%s", found_name,
+					      state->end+1);
+		}
+		if (tmp == NULL) {
+			DBG_ERR("talloc_asprintf failed\n");
+			return NT_STATUS_NO_MEMORY;
 		}
+		TALLOC_FREE(state->smb_fname->base_name);
+		state->smb_fname->base_name = tmp;
+		state->name = state->smb_fname->base_name + name_ofs;
+		state->end = state->name + strlen(found_name);
+		*state->end = '\0';
+	} else {
+		char *tmp;
+		size_t name_ofs =
+			state->name - state->smb_fname->base_name;
+
+		if (!ISDOT(state->dirpath)) {
+			tmp = talloc_asprintf(state->smb_fname,
+					      "%s/%s", state->dirpath,
+					      found_name);
+		} else {
+			tmp = talloc_strdup(state->smb_fname,
+					    found_name);
+		}
+		if (tmp == NULL) {
+			DBG_ERR("talloc failed\n");
+			return NT_STATUS_NO_MEMORY;
+		}
+		TALLOC_FREE(state->smb_fname->base_name);
+		state->smb_fname->base_name = tmp;
+		state->name = state->smb_fname->base_name + name_ofs;
+
+		/*
+		 * We just scanned for, and found the end of
+		 * the path. We must return a valid stat struct
+		 * if it exists. JRA.
+		 */
 
-		TALLOC_FREE(found_name);
-	} /* end else */
+		if (state->posix_pathnames) {
+			ret = SMB_VFS_LSTAT(state->conn, state->smb_fname);
+		} else {
+			ret = SMB_VFS_STAT(state->conn, state->smb_fname);
+		}
+
+		if (ret != 0) {
+			SET_STAT_INVALID(state->smb_fname->st);
+		}
+	}
 
+	TALLOC_FREE(found_name);
 	return NT_STATUS_OK;
 }