From 006eee8e923dac2b34ead6d6277907668ab6457c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 31 May 2019 14:46:16 -0700 Subject: [PATCH] WIP: prototype fix for mkdir races. Signed-off-by: Jeremy Allison --- 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