The branch, master has been updated via df7efdf0465 s3: smbd: Cleanup - Make rmdir_internals() use NTSTATUS internally without depending on errno. via 28522bb3771 s3: smbd: Cleanup - make recursive_rmdir() return a more expressive NTSTATUS not bool. via b3514a57e9b smbd: Make complex if-expression in file_set_dosmode() easier to read via ab692aa6e70 smbd: Fix indentation in rename_internals_fsp() via 5567d5bca29 smbd: Save a few lines in file_set_dosmode() with "goto done;" via 2976177005f smbd: Remove unused "lret" variable from file_set_dosmode() via f60ca2e2f35 smbd: Pass dirfsp instead of a parent filename to unix_mode via be6cc4cc23f smbd: Log close_file_free() failure in copy_internals() via fbb4bd365f1 smbd: Pass dirfsp instead of an fname to open_file() via fd1dca2d175 smbd: Inherit acl from an fsp instead of a fname via d1a0862327f smbd: Remove a deref forgotten in c2ac6a9cd7b from e25d6c89bef WHATSNEW: Bronze bit, S4U and RBDC support with MIT Kerberos 1.20
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit df7efdf046504aa2392a53f8fd96de9c207f854c Author: Jeremy Allison <j...@samba.org> Date: Thu Mar 3 09:49:15 2022 -0800 s3: smbd: Cleanup - Make rmdir_internals() use NTSTATUS internally without depending on errno. As we already need to return NTSTATUS, map errno to NTSTATUS directly at point of failure and don't depend on keeping it around. No change in client-visible behavior but makes rmdir_internals() easier to understand (for me at least). Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> Autobuild-User(master): Ralph Böhme <s...@samba.org> Autobuild-Date(master): Fri Mar 4 18:39:48 UTC 2022 on sn-devel-184 commit 28522bb3771245ae69d7c9e279214b1f8ad2c526 Author: Jeremy Allison <j...@samba.org> Date: Thu Mar 3 09:34:45 2022 -0800 s3: smbd: Cleanup - make recursive_rmdir() return a more expressive NTSTATUS not bool. Next cleanup the internals of rmdir_internals() to do an early map of errno -> NTSTATUS to avoid mapping back and forth. Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit b3514a57e9b9b35bc9983d997191c575eeebcf85 Author: Volker Lendecke <v...@samba.org> Date: Fri Mar 4 08:39:01 2022 +0100 smbd: Make complex if-expression in file_set_dosmode() easier to read Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit ab692aa6e706a23722e1d3f538582d8394507adb Author: Volker Lendecke <v...@samba.org> Date: Fri Mar 4 08:36:04 2022 +0100 smbd: Fix indentation in rename_internals_fsp() This one space character makes it more obvious where in the copmlex if-expression lp_store_dos_attributes() lives. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 5567d5bca2963534dcc4fb1728f83f18d42c9691 Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 21:49:47 2022 +0100 smbd: Save a few lines in file_set_dosmode() with "goto done;" Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit 2976177005feff38f6ef6da1ae0733041849be2b Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 21:48:26 2022 +0100 smbd: Remove unused "lret" variable from file_set_dosmode() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit f60ca2e2f35666583f2e8cd11cb507406bb17393 Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 11:52:12 2022 +0100 smbd: Pass dirfsp instead of a parent filename to unix_mode This converts a STAT (with potential symlink race problems) into an FSTAT on the O_PATH fd we have for the directory Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit be6cc4cc23f61d4c44796621daf726733f718a1a Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 20:13:25 2022 +0100 smbd: Log close_file_free() failure in copy_internals() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit fbb4bd365f156fef89e96f7b79040443f0d70d0a Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 11:32:20 2022 +0100 smbd: Pass dirfsp instead of an fname to open_file() Moving slowly towards passing directory handles instead of names, representing the idea that we hold a O_PATH file descriptor on directories. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit fd1dca2d175291f2258f7963419b16ea3f5c4e31 Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 11:32:20 2022 +0100 smbd: Inherit acl from an fsp instead of a fname Moving slowly towards passing directory handles instead of names, representing the idea that we hold a O_PATH file descriptor on directories. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> commit d1a0862327f37f2edd1042b3b66c2e85234b1e94 Author: Volker Lendecke <v...@samba.org> Date: Thu Mar 3 11:28:57 2022 +0100 smbd: Remove a deref forgotten in c2ac6a9cd7b Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/smbd/close.c | 72 ++++++++++++++++++++++++----------------------- source3/smbd/dosmode.c | 52 +++++++++++++++++----------------- source3/smbd/nttrans.c | 8 ++++++ source3/smbd/open.c | 31 +++++++++++--------- source3/smbd/posix_acls.c | 10 +++---- source3/smbd/proto.h | 10 +++---- source3/smbd/reply.c | 2 +- source3/smbd/trans2.c | 2 +- 8 files changed, 100 insertions(+), 87 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/smbd/close.c b/source3/smbd/close.c index 798a28c324c..a4bdd56729a 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -839,19 +839,18 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp, tree recursively. Return True on ok, False on fail. ****************************************************************************/ -bool recursive_rmdir(TALLOC_CTX *ctx, +NTSTATUS recursive_rmdir(TALLOC_CTX *ctx, connection_struct *conn, struct smb_filename *smb_dname) { const char *dname = NULL; char *talloced = NULL; - bool ret = True; long offset = 0; SMB_STRUCT_STAT st; struct smb_Dir *dir_hnd = NULL; struct files_struct *dirfsp = NULL; int retval; - NTSTATUS status; + NTSTATUS status = NT_STATUS_OK; SMB_ASSERT(!is_ntfs_stream_smb_fname(smb_dname)); @@ -862,8 +861,7 @@ bool recursive_rmdir(TALLOC_CTX *ctx, 0, &dir_hnd); if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); - return False; + return status; } dirfsp = dir_hnd_fetch_fsp(dir_hnd); @@ -886,7 +884,7 @@ bool recursive_rmdir(TALLOC_CTX *ctx, smb_dname->base_name, dname); if (!fullname) { - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err_break; } @@ -897,16 +895,18 @@ bool recursive_rmdir(TALLOC_CTX *ctx, smb_dname->twrp, smb_dname->flags); if (smb_dname_full == NULL) { - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err_break; } if (SMB_VFS_LSTAT(conn, smb_dname_full) != 0) { + status = map_nt_error_from_unix(errno); goto err_break; } if (smb_dname_full->st.st_ex_mode & S_IFDIR) { - if (!recursive_rmdir(ctx, conn, smb_dname_full)) { + status = recursive_rmdir(ctx, conn, smb_dname_full); + if (!NT_STATUS_IS_OK(status)) { goto err_break; } unlink_flags = AT_REMOVEDIR; @@ -921,7 +921,6 @@ bool recursive_rmdir(TALLOC_CTX *ctx, smb_dname_full->flags, &atname); if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); goto err_break; } @@ -938,6 +937,7 @@ bool recursive_rmdir(TALLOC_CTX *ctx, atname, unlink_flags); if (retval != 0) { + status = map_nt_error_from_unix(errno); goto err_break; } @@ -950,12 +950,11 @@ bool recursive_rmdir(TALLOC_CTX *ctx, TALLOC_FREE(talloced); TALLOC_FREE(atname); if (do_break) { - ret = false; break; } } TALLOC_FREE(dir_hnd); - return ret; + return status; } /**************************************************************************** @@ -1043,7 +1042,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) */ if (!lp_delete_veto_files(SNUM(conn))) { - errno = ENOTEMPTY; + status = NT_STATUS_DIRECTORY_NOT_EMPTY; goto err; } @@ -1064,7 +1063,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) * to avoid leaking information about what we * can't delete. */ - errno = ENOTEMPTY; + status = NT_STATUS_DIRECTORY_NOT_EMPTY; goto err; } @@ -1092,7 +1091,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) if (fullname == NULL) { TALLOC_FREE(talloced); - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err; } @@ -1105,17 +1104,16 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) if (smb_dname_full == NULL) { TALLOC_FREE(talloced); TALLOC_FREE(fullname); - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err; } retval = SMB_VFS_LSTAT(conn, smb_dname_full); if (retval != 0) { - int saved_errno = errno; + status = map_nt_error_from_unix(errno); TALLOC_FREE(talloced); TALLOC_FREE(fullname); TALLOC_FREE(smb_dname_full); - errno = saved_errno; goto err; } @@ -1134,7 +1132,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) TALLOC_FREE(talloced); TALLOC_FREE(fullname); TALLOC_FREE(smb_dname_full); - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err; } if (is_msdfs_link(fsp, smb_atname)) { @@ -1146,7 +1144,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) "- can't delete directory %s\n", dname, fsp_str_dbg(fsp)); - errno = ENOTEMPTY; + status = NT_STATUS_DIRECTORY_NOT_EMPTY; goto err; } TALLOC_FREE(smb_atname); @@ -1172,7 +1170,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) TALLOC_FREE(talloced); TALLOC_FREE(fullname); TALLOC_FREE(smb_dname_full); - errno = ENOTEMPTY; + status = NT_STATUS_DIRECTORY_NOT_EMPTY; goto err; } @@ -1189,7 +1187,6 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) TALLOC_FREE(talloced); TALLOC_FREE(fullname); TALLOC_FREE(smb_dname_full); - errno = map_errno_from_nt_status(status); goto err; } @@ -1213,7 +1210,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) TALLOC_FREE(fullname); TALLOC_FREE(smb_dname_full); TALLOC_FREE(direntry_fname); - errno = ENOTEMPTY; + status = NT_STATUS_DIRECTORY_NOT_EMPTY; goto err; } @@ -1238,7 +1235,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) dname); if (fullname == NULL) { - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err_break; } @@ -1249,7 +1246,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) smb_dname->twrp, smb_dname->flags); if (smb_dname_full == NULL) { - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err_break; } @@ -1259,6 +1256,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) retval = SMB_VFS_LSTAT(conn, smb_dname_full); if (retval != 0) { + status = map_nt_error_from_unix(errno); goto err_break; } @@ -1276,7 +1274,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) smb_dname->twrp, smb_dname->flags); if (direntry_fname == NULL) { - errno = ENOMEM; + status = NT_STATUS_NO_MEMORY; goto err_break; } } else { @@ -1289,7 +1287,6 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) smb_dname->flags, &direntry_fname); if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); goto err_break; } @@ -1305,9 +1302,8 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) unlink_flags = 0; if (smb_dname_full->st.st_ex_mode & S_IFDIR) { - if (!recursive_rmdir(ctx, conn, - smb_dname_full)) - { + status = recursive_rmdir(ctx, conn, smb_dname_full); + if (!NT_STATUS_IS_OK(status)) { goto err_break; } unlink_flags = AT_REMOVEDIR; @@ -1318,6 +1314,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) direntry_fname, unlink_flags); if (retval != 0) { + status = map_nt_error_from_unix(errno); goto err_break; } @@ -1334,30 +1331,35 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) } } + /* If we get here, we know NT_STATUS_IS_OK(status) */ + SMB_ASSERT(NT_STATUS_IS_OK(status)); + /* Retry the rmdir */ ret = SMB_VFS_UNLINKAT(conn, parent_fname->fsp, at_fname, AT_REMOVEDIR); - + if (ret != 0) { + status = map_nt_error_from_unix(errno); + } err: TALLOC_FREE(dir_hnd); TALLOC_FREE(parent_fname); - if (ret != 0) { - DEBUG(3,("rmdir_internals: couldn't remove directory %s : " + if (!NT_STATUS_IS_OK(status)) { + DBG_NOTICE("couldn't remove directory %s : " "%s\n", smb_fname_str_dbg(smb_dname), - strerror(errno))); - return map_nt_error_from_unix(errno); + nt_errstr(status)); + return status; } notify_fname(conn, NOTIFY_ACTION_REMOVED, FILE_NOTIFY_CHANGE_DIR_NAME, smb_dname->base_name); - return NT_STATUS_OK; + return status; } /**************************************************************************** diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index 68764b46ac8..43bb48d38c5 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -106,7 +106,7 @@ static uint32_t filter_mode_by_protocol(uint32_t mode) mode_t unix_mode(connection_struct *conn, int dosmode, const struct smb_filename *smb_fname, - struct smb_filename *smb_fname_parent) + struct files_struct *parent_dirfsp) { mode_t result = (S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH); mode_t dir_mode = 0; /* Mode of the inherit_from directory if @@ -116,20 +116,24 @@ mode_t unix_mode(connection_struct *conn, int dosmode, result &= ~(S_IWUSR | S_IWGRP | S_IWOTH); } - if ((smb_fname_parent != NULL) && lp_inherit_permissions(SNUM(conn))) { + if ((parent_dirfsp != NULL) && lp_inherit_permissions(SNUM(conn))) { + struct stat_ex sbuf = { .st_ex_nlink = 0, }; + int ret; + DBG_DEBUG("[%s] inheriting from [%s]\n", smb_fname_str_dbg(smb_fname), - smb_fname_str_dbg(smb_fname_parent)); + smb_fname_str_dbg(parent_dirfsp->fsp_name)); - if (SMB_VFS_STAT(conn, smb_fname_parent) != 0) { - DBG_ERR("stat failed [%s]: %s\n", - smb_fname_str_dbg(smb_fname_parent), + ret = SMB_VFS_FSTAT(parent_dirfsp, &sbuf); + if (ret != 0) { + DBG_ERR("fstat failed [%s]: %s\n", + smb_fname_str_dbg(parent_dirfsp->fsp_name), strerror(errno)); return(0); /* *** shouldn't happen! *** */ } /* Save for later - but explicitly remove setuid bit for safety. */ - dir_mode = smb_fname_parent->st.st_ex_mode & ~S_ISUID; + dir_mode = sbuf.st_ex_mode & ~S_ISUID; DEBUG(2,("unix_mode(%s) inherit mode %o\n", smb_fname_str_dbg(smb_fname), (int)dir_mode)); /* Clear "result" */ @@ -919,7 +923,7 @@ int file_set_dosmode(connection_struct *conn, int mask=0; mode_t tmp; mode_t unixmode; - int ret = -1, lret = -1; + int ret = -1; NTSTATUS status; if (!CAN_WRITE(conn)) { @@ -954,13 +958,8 @@ int file_set_dosmode(connection_struct *conn, } if (NT_STATUS_IS_OK(status)) { - if (!newfile) { - notify_fname(conn, NOTIFY_ACTION_MODIFIED, - FILE_NOTIFY_CHANGE_ATTRIBUTES, - smb_fname->base_name); - } - smb_fname->st.st_ex_mode = unixmode; - return 0; + ret = 0; + goto done; } /* @@ -973,7 +972,11 @@ int file_set_dosmode(connection_struct *conn, } /* Fall back to UNIX modes. */ - unixmode = unix_mode(conn, dosmode, smb_fname, parent_dir); + unixmode = unix_mode( + conn, + dosmode, + smb_fname, + parent_dir != NULL ? parent_dir->fsp : NULL); /* preserve the file type bits */ mask |= S_IFMT; @@ -1019,9 +1022,11 @@ int file_set_dosmode(connection_struct *conn, * Simply refuse to do the chmod in this case. */ - if (S_ISDIR(smb_fname->st.st_ex_mode) && (unixmode & S_ISGID) && - geteuid() != sec_initial_uid() && - !current_user_in_group(conn, smb_fname->st.st_ex_gid)) { + if (S_ISDIR(smb_fname->st.st_ex_mode) && + (unixmode & S_ISGID) && + geteuid() != sec_initial_uid() && + !current_user_in_group(conn, smb_fname->st.st_ex_gid)) + { DEBUG(3,("file_set_dosmode: setgid bit cannot be " "set for directory %s\n", smb_fname_str_dbg(smb_fname))); @@ -1031,13 +1036,7 @@ int file_set_dosmode(connection_struct *conn, ret = SMB_VFS_FCHMOD(smb_fname->fsp, unixmode); if (ret == 0) { - if(!newfile || (lret != -1)) { - notify_fname(conn, NOTIFY_ACTION_MODIFIED, - FILE_NOTIFY_CHANGE_ATTRIBUTES, - smb_fname->base_name); - } - smb_fname->st.st_ex_mode = unixmode; - return 0; + goto done; } if((errno != EPERM) && (errno != EACCES)) @@ -1060,6 +1059,7 @@ int file_set_dosmode(connection_struct *conn, ret = SMB_VFS_FCHMOD(smb_fname->fsp, unixmode); unbecome_root(); +done: if (!newfile) { notify_fname(conn, NOTIFY_ACTION_MODIFIED, FILE_NOTIFY_CHANGE_ATTRIBUTES, diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index af7a49646e9..0fa18eb88e1 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1663,6 +1663,14 @@ NTSTATUS copy_internals(TALLOC_CTX *ctx, set_close_write_time(fsp2, smb_fname_src->st.st_ex_mtime); status = close_file_free(NULL, &fsp2, NORMAL_CLOSE); + if (!NT_STATUS_IS_OK(status)) { + DBG_WARNING("close_file_free() failed: %s\n", + nt_errstr(status)); + /* + * We can't do much but leak the fsp + */ + goto out; + } /* 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 index 2f7fcfa772a..baae51f157e 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1066,7 +1066,7 @@ static NTSTATUS fd_open_atomic(struct files_struct *dirfsp, { NTSTATUS status = NT_STATUS_UNSUCCESSFUL; NTSTATUS retry_status; - bool file_existed = VALID_STAT(fsp->fsp_name->st); + bool file_existed = VALID_STAT(smb_fname->st); int curr_flags; if (!(flags & O_CREAT)) { @@ -1260,7 +1260,7 @@ static NTSTATUS reopen_from_fsp(struct files_struct *fsp, static NTSTATUS open_file(files_struct *fsp, struct smb_request *req, - struct smb_filename *parent_dir, + struct files_struct *dirfsp, int flags, mode_t unx_mode, uint32_t access_mask, /* client requested access mask. */ @@ -1383,7 +1383,7 @@ static NTSTATUS open_file(files_struct *fsp, /* Only do this check on non-stream open. */ if (file_existed) { status = smbd_check_access_rights_fsp( - parent_dir->fsp, + dirfsp, fsp, false, access_mask); @@ -1421,13 +1421,14 @@ static NTSTATUS open_file(files_struct *fsp, } status = check_parent_access_fsp( - parent_dir->fsp, + dirfsp, SEC_DIR_ADD_FILE); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("check_parent_access_fsp on " "directory %s for file %s " "returned %s\n", - smb_fname_str_dbg(parent_dir), + smb_fname_str_dbg( + dirfsp->fsp_name), smb_fname_str_dbg(smb_fname), nt_errstr(status)); return status; @@ -1490,7 +1491,7 @@ static NTSTATUS open_file(files_struct *fsp, /* Inherit the ACL if required */ if (lp_inherit_permissions(SNUM(conn))) { inherit_access_posix_acl(conn, - parent_dir, + dirfsp, smb_fname, unx_mode); need_re_stat = true; @@ -1498,8 +1499,7 @@ static NTSTATUS open_file(files_struct *fsp, /* Change the owner if required. */ if (lp_inherit_owner(SNUM(conn)) != INHERIT_OWNER_NO) { - change_file_owner_to_parent_fsp(parent_dir->fsp, - fsp); + change_file_owner_to_parent_fsp(dirfsp, fsp); need_re_stat = true; } @@ -1567,7 +1567,7 @@ static NTSTATUS open_file(files_struct *fsp, } -- Samba Shared Repository