The branch, master has been updated via da1103dbdc9 smbd: build smb_fname per file to delete in unlink_internals() via f31108cd7dc smbd: consolidate fsp allocation for open_directory() and open_file_ntcreate() via 09cfac6565c smbd: factor out fsp_bind_smb() via 9c187e29009 smbd: don't mess with smb_dname->base_name in call_trans2findfirst() via 567f7987e39 s3/lib: fsp_str_dbg() doesn't show a possible stream name via 8c2c749157a smbd: use helper variable for fd in fd_open() via 92d8b5f5ed3 smbd: remove errno saving from fd_open() via 1508d661cbd smbd: remove a nested block in non_widelink_open() via b6e40dbf9bd smbd: use (global) POSIX pathname state in non_widelink_open() via afdeba88194 smbd: check for conn->cwd_fsp in fd_close() via a929644b28c smbd: check for conn->cwd_fsp in file_free() via 6ab75d2c629 smbd: use a helper variable in open_file() via 916ebade769 smbd: remove unused NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE via 4523a0b47d5 smbd: ensure we do a base open for internal stream deletes from 63b9b2a103a vfs_posixacl: Remove unnecessary call to acl_set_permset
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit da1103dbdc9d9901ff1e95d6a0d81db53c6ebcec Author: Ralph Boehme <s...@samba.org> Date: Fri Jun 19 12:31:23 2020 +0200 smbd: build smb_fname per file to delete in unlink_internals() Make sure to pass fresh smb_fname's to do_unlink(). Needed for path-ref fsps in the future. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Sat Jul 18 07:21:11 UTC 2020 on sn-devel-184 commit f31108cd7dc043b7094041363275f92b4390e0fc Author: Ralph Boehme <s...@samba.org> Date: Fri Jun 19 09:40:37 2020 +0200 smbd: consolidate fsp allocation for open_directory() and open_file_ntcreate() ...at one place in the caller create_file_unixpath(). Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 09cfac6565c42d97d9d8cde6a3bf255c41158c4f Author: Ralph Boehme <s...@samba.org> Date: Fri Jun 19 07:31:51 2020 +0200 smbd: factor out fsp_bind_smb() Needed for path-ref fsps in the future. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 9c187e2900915b106af15da0dd7a2189877f0acb Author: Ralph Boehme <s...@samba.org> Date: Thu Jun 18 10:54:30 2020 +0200 smbd: don't mess with smb_dname->base_name in call_trans2findfirst() Create a fresh name instead. Needed to proper support for path-ref fsps in the future. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 567f7987e39ffc73ef30e5ef0da0ffdc990b3907 Author: Ralph Boehme <s...@samba.org> Date: Tue Jun 23 12:14:48 2020 +0200 s3/lib: fsp_str_dbg() doesn't show a possible stream name Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 8c2c749157aac80317e9de34cf99e779c0af87fe Author: Ralph Boehme <s...@samba.org> Date: Wed Jul 15 12:04:10 2020 +0200 smbd: use helper variable for fd in fd_open() No change in behaviour. Fwiw, no need to set fsp->fh->fd to -1 in the error case, as that is initialized to -1 in fsp_new(). Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 92d8b5f5ed3910546183b5663e31d6b6c44ad6cc Author: Ralph Boehme <s...@samba.org> Date: Wed Jul 15 12:01:18 2020 +0200 smbd: remove errno saving from fd_open() This is not needed anymore since 97d061237b0f2e07ae9c8e893734e222e58cfa4e. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 1508d661cbd84bbe7d72b764c529a6214da03bdd Author: Ralph Boehme <s...@samba.org> Date: Thu Jul 2 08:02:45 2020 +0200 smbd: remove a nested block in non_widelink_open() No change in behaviour. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit b6e40dbf9bd5a35bfe98fbe7e570d6742722dc57 Author: Ralph Boehme <s...@samba.org> Date: Thu Jul 2 07:57:57 2020 +0200 smbd: use (global) POSIX pathname state in non_widelink_open() Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit afdeba88194bc1b2ae66dd5e598211801698c189 Author: Ralph Boehme <s...@samba.org> Date: Sat Jun 13 16:22:20 2020 +0200 smbd: check for conn->cwd_fsp in fd_close() This avoids doing the check in all callers. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit a929644b28c6dc71594c1000cfaeeb0fde094184 Author: Ralph Boehme <s...@samba.org> Date: Sat Jun 13 16:21:22 2020 +0200 smbd: check for conn->cwd_fsp in file_free() This avoids doing the check in all callers. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 6ab75d2c629969f1b654c7228eb7fb050eeb615d Author: Ralph Boehme <s...@samba.org> Date: Sun Jun 7 16:42:50 2020 +0200 smbd: use a helper variable in open_file() No change in behaviour, this just prepares for changes to come. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 916ebade769a55eb6d87bf9078533e41d3497683 Author: Ralph Boehme <s...@samba.org> Date: Mon Jun 8 09:29:01 2020 +0200 smbd: remove unused NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE The previous commit removed the only consumer of the flags. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 4523a0b47d56423216a052df765e39a2389e80e2 Author: Ralph Boehme <s...@samba.org> Date: Mon Jun 8 09:26:41 2020 +0200 smbd: ensure we do a base open for internal stream deletes Otherwise we're not opening the basefile so fsp->base_fsp remains NULL for fsp handles on streams. As there are some places that use the check (fsp->base_fsp != NULL) to check for stream handles, eg streams_xattr_fstat(), we must ensure it is set otherwise we open a pretty big window for undefined behaviour. Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/include/smb.h | 3 - source3/lib/filename_util.c | 7 +- source3/smbd/files.c | 64 +++++++++++------- source3/smbd/open.c | 156 +++++++++++++++++--------------------------- source3/smbd/proto.h | 1 + source3/smbd/reply.c | 36 ++++++---- source3/smbd/trans2.c | 15 +++-- 7 files changed, 142 insertions(+), 140 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/include/smb.h b/source3/include/smb.h index 311490d699e..d2eabc63e9c 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -409,9 +409,6 @@ Offset Data length. #define NTCREATEX_OPTIONS_PRIVATE_DENY_DOS 0x0001 #define NTCREATEX_OPTIONS_PRIVATE_DENY_FCB 0x0002 -/* Private options for streams support */ -#define NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE 0x0004 - /* Private options for printer support */ #define NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE 0x0008 diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c index 418dd20327b..84a248eb766 100644 --- a/source3/lib/filename_util.c +++ b/source3/lib/filename_util.c @@ -195,10 +195,15 @@ const char *fsp_str_dbg(const struct files_struct *fsp) return name; } + name = smb_fname_str_dbg(fsp->fsp_name); + if (name == NULL) { + return ""; + } + name = talloc_asprintf(talloc_tos(), "%s/%s", fsp->dirfsp->fsp_name->base_name, - fsp->fsp_name->base_name); + name); if (name == NULL) { return ""; } diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 1a59db7d1e1..999100a9e6c 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -99,6 +99,39 @@ void fsp_set_gen_id(files_struct *fsp) Find first available file slot. ****************************************************************************/ +NTSTATUS fsp_bind_smb(struct files_struct *fsp, struct smb_request *req) +{ + struct smbXsrv_open *op = NULL; + NTTIME now; + NTSTATUS status; + + if (req == NULL) { + DBG_DEBUG("INTERNAL_OPEN_ONLY, skipping smbXsrv_open\n"); + return NT_STATUS_OK; + } + + now = timeval_to_nttime(&fsp->open_time); + + status = smbXsrv_open_create(req->xconn, + fsp->conn->session_info, + now, + &op); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + fsp->op = op; + op->compat = fsp; + fsp->fnum = op->local_id; + + fsp->mid = req->mid; + req->chain_fsp = fsp; + + DBG_DEBUG("fsp [%s] mid [%" PRIu64"]\n", + fsp_str_dbg(fsp), fsp->mid); + + return NT_STATUS_OK; +} + NTSTATUS file_new(struct smb_request *req, connection_struct *conn, files_struct **result) { @@ -113,24 +146,10 @@ NTSTATUS file_new(struct smb_request *req, connection_struct *conn, GetTimeOfDay(&fsp->open_time); - if (req) { - struct smbXsrv_connection *xconn = req->xconn; - struct smbXsrv_open *op = NULL; - NTTIME now = timeval_to_nttime(&fsp->open_time); - - status = smbXsrv_open_create(xconn, - conn->session_info, - now, &op); - if (!NT_STATUS_IS_OK(status)) { - file_free(NULL, fsp); - return status; - } - fsp->op = op; - op->compat = fsp; - fsp->fnum = op->local_id; - } else { - DEBUG(10, ("%s: req==NULL, INTERNAL_OPEN_ONLY, smbXsrv_open " - "allocated\n", __func__)); + status = fsp_bind_smb(fsp, req); + if (!NT_STATUS_IS_OK(status)) { + file_free(NULL, fsp); + return status; } fsp_set_gen_id(fsp); @@ -154,11 +173,6 @@ NTSTATUS file_new(struct smb_request *req, connection_struct *conn, DEBUG(5,("allocated file structure %s (%u used)\n", fsp_fnum_dbg(fsp), (unsigned int)sconn->num_files)); - if (req != NULL) { - fsp->mid = req->mid; - req->chain_fsp = fsp; - } - /* A new fsp invalidates the positive and negative fsp_fi_cache as the new fsp is pushed at the start of the list and we search from @@ -591,6 +605,10 @@ void file_free(struct smb_request *req, files_struct *fsp) struct smbd_server_connection *sconn = fsp->conn->sconn; uint64_t fnum = fsp->fnum; + if (fsp == fsp->conn->cwd_fsp) { + return; + } + if (fsp->notify) { size_t len = fsp_fullbasepath(fsp, NULL, 0); char fullpath[len+1]; diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 79b72784249..17d6823bcb8 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -628,6 +628,7 @@ static int non_widelink_open(files_struct *fsp, struct connection_struct *conn = fsp->conn; NTSTATUS status; int fd = -1; + struct smb_filename *tmp_fsp_name = fsp->fsp_name; struct smb_filename *smb_fname_rel = NULL; int saved_errno = 0; struct smb_filename *oldwd_fname = NULL; @@ -690,20 +691,16 @@ static int non_widelink_open(files_struct *fsp, flags |= O_NOFOLLOW; - { - struct smb_filename *tmp_name = fsp->fsp_name; - - fsp->fsp_name = smb_fname_rel; + fsp->fsp_name = smb_fname_rel; - fd = SMB_VFS_OPENAT(conn, - cwdfsp, - smb_fname_rel, - fsp, - flags, - mode); + fd = SMB_VFS_OPENAT(conn, + cwdfsp, + smb_fname_rel, + fsp, + flags, + mode); - fsp->fsp_name = tmp_name; - } + fsp->fsp_name = tmp_fsp_name; if (fd == -1) { saved_errno = link_errno_convert(errno); @@ -719,7 +716,7 @@ static int non_widelink_open(files_struct *fsp, * BUG: https://bugzilla.samba.org/show_bug.cgi?id=12860 */ if (saved_errno == ELOOP || saved_errno == ENOTDIR) { - if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) { + if (fsp->fsp_name->flags & SMB_FILENAME_POSIX_PATH) { /* Never follow symlinks on posix open. */ goto out; } @@ -782,7 +779,7 @@ NTSTATUS fd_open(files_struct *fsp, struct connection_struct *conn = fsp->conn; struct smb_filename *smb_fname = fsp->fsp_name; NTSTATUS status = NT_STATUS_OK; - int saved_errno = 0; + int fd; /* * Never follow symlinks on a POSIX client. The @@ -797,19 +794,8 @@ NTSTATUS fd_open(files_struct *fsp, * Only follow symlinks within a share * definition. */ - fsp->fh->fd = non_widelink_open(fsp, - smb_fname, - flags, - mode, - 0); - if (fsp->fh->fd == -1) { - saved_errno = errno; - } - if (saved_errno != 0) { - errno = saved_errno; - } - - if (fsp->fh->fd == -1) { + fd = non_widelink_open(fsp, smb_fname, flags, mode, 0); + if (fd == -1) { int posix_errno = link_errno_convert(errno); status = map_nt_error_from_unix(posix_errno); if (errno == EMFILE) { @@ -826,12 +812,14 @@ NTSTATUS fd_open(files_struct *fsp, DBG_DEBUG("name %s, flags = 0%o mode = 0%o, fd = %d. %s\n", smb_fname_str_dbg(smb_fname), flags, (int)mode, - fsp->fh->fd, strerror(errno)); + fd, strerror(errno)); return status; } + fsp->fh->fd = fd; + DBG_DEBUG("name %s, flags = 0%o mode = 0%o, fd = %d\n", - smb_fname_str_dbg(smb_fname), flags, (int)mode, fsp->fh->fd); + smb_fname_str_dbg(smb_fname), flags, (int)mode, fd); return status; } @@ -908,6 +896,10 @@ NTSTATUS fd_close(files_struct *fsp) { int ret; + if (fsp == fsp->conn->cwd_fsp) { + return NT_STATUS_OK; + } + if (fsp->dptr) { dptr_CloseDir(fsp); } @@ -1221,6 +1213,7 @@ static NTSTATUS open_file(files_struct *fsp, READ_CONTROL_ACCESS; bool creating = !file_existed && (flags & O_CREAT); bool truncating = (flags & O_TRUNC); + bool open_fd = false; fsp->fh->fd = -1; errno = EPERM; @@ -1273,6 +1266,10 @@ static NTSTATUS open_file(files_struct *fsp, } if ((open_access_mask & need_fd_mask) || creating || truncating) { + open_fd = true; + } + + if (open_fd) { const char *wild; int ret; @@ -4378,16 +4375,15 @@ static NTSTATUS mkdir_internal(connection_struct *conn, static NTSTATUS open_directory(connection_struct *conn, struct smb_request *req, struct files_struct **dirfsp, - struct smb_filename *smb_dname, uint32_t access_mask, uint32_t share_access, uint32_t create_disposition, uint32_t create_options, uint32_t file_attributes, int *pinfo, - files_struct **result) + struct files_struct *fsp) { - files_struct *fsp = NULL; + struct smb_filename *smb_dname = fsp->fsp_name; bool dir_existed = VALID_STAT(smb_dname->st); struct share_mode_lock *lck = NULL; NTSTATUS status; @@ -4563,11 +4559,6 @@ static NTSTATUS open_directory(connection_struct *conn, } } - status = file_new(req, conn, &fsp); - if(!NT_STATUS_IS_OK(status)) { - return status; - } - /* * Setup the files_struct for it. */ @@ -4592,17 +4583,6 @@ static NTSTATUS open_directory(connection_struct *conn, if (file_attributes & FILE_FLAG_POSIX_SEMANTICS) { fsp->posix_flags |= FSP_POSIX_FLAGS_ALL; } - status = fsp_set_smb_fname(fsp, smb_dname); - if (!NT_STATUS_IS_OK(status)) { - file_free(req, fsp); - return status; - } - - if (*dirfsp == fsp->conn->cwd_fsp) { - fsp->dirfsp = fsp->conn->cwd_fsp; - } else { - fsp->dirfsp = talloc_move(fsp, dirfsp); - } /* Don't store old timestamps for directory handles in the internal database. We don't @@ -4625,14 +4605,12 @@ static NTSTATUS open_directory(connection_struct *conn, "%s (%s)\n", smb_fname_str_dbg(smb_dname), nt_errstr(status)); - file_free(req, fsp); return status; } status = vfs_stat_fsp(fsp); if (!NT_STATUS_IS_OK(status)) { fd_close(fsp); - file_free(req, fsp); return status; } @@ -4640,7 +4618,6 @@ static NTSTATUS open_directory(connection_struct *conn, DEBUG(5,("open_directory: %s is not a directory !\n", smb_fname_str_dbg(smb_dname))); fd_close(fsp); - file_free(req, fsp); return NT_STATUS_NOT_A_DIRECTORY; } @@ -4652,7 +4629,6 @@ static NTSTATUS open_directory(connection_struct *conn, "directory %s.\n", smb_fname_str_dbg(smb_dname))); fd_close(fsp); - file_free(req, fsp); return NT_STATUS_ACCESS_DENIED; } @@ -4664,14 +4640,12 @@ static NTSTATUS open_directory(connection_struct *conn, DEBUG(0, ("open_directory: Could not get share mode lock for " "%s\n", smb_fname_str_dbg(smb_dname))); fd_close(fsp); - file_free(req, fsp); return NT_STATUS_SHARING_VIOLATION; } if (has_delete_on_close(lck, fsp->name_hash)) { TALLOC_FREE(lck); fd_close(fsp); - file_free(req, fsp); return NT_STATUS_DELETE_PENDING; } @@ -4681,7 +4655,6 @@ static NTSTATUS open_directory(connection_struct *conn, if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(lck); fd_close(fsp); - file_free(req, fsp); return status; } @@ -4707,7 +4680,6 @@ static NTSTATUS open_directory(connection_struct *conn, if (!ok) { TALLOC_FREE(lck); fd_close(fsp); - file_free(req, fsp); return NT_STATUS_NO_MEMORY; } @@ -4719,7 +4691,6 @@ static NTSTATUS open_directory(connection_struct *conn, del_share_mode(lck, fsp); TALLOC_FREE(lck); fd_close(fsp); - file_free(req, fsp); return status; } @@ -4748,7 +4719,6 @@ static NTSTATUS open_directory(connection_struct *conn, *pinfo = info; } - *result = fsp; return NT_STATUS_OK; } @@ -4965,7 +4935,7 @@ static NTSTATUS open_streams_for_delete(connection_struct *conn, 0, /* oplock_request */ NULL, /* lease */ 0, /* allocation_size */ - NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE, /* private_flags */ + 0, /* private_flags */ NULL, /* sd */ NULL, /* ea_list */ &streams[i], /* result */ @@ -5649,8 +5619,8 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, } if ((conn->fs_capabilities & FILE_NAMED_STREAMS) - && is_ntfs_stream_smb_fname(smb_fname) - && (!(private_flags & NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE))) { + && is_ntfs_stream_smb_fname(smb_fname)) + { uint32_t base_create_disposition; struct smb_filename *smb_fname_base = NULL; uint32_t base_privflags; @@ -5739,6 +5709,31 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, fd_close(base_fsp); } + status = file_new(req, conn, &fsp); + if(!NT_STATUS_IS_OK(status)) { + goto fail; + } + + status = fsp_set_smb_fname(fsp, smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + + if (*dirfsp == fsp->conn->cwd_fsp) { + fsp->dirfsp = fsp->conn->cwd_fsp; + } else { + fsp->dirfsp = talloc_move(fsp, dirfsp); + } + + if (base_fsp) { + /* + * We're opening the stream element of a + * base_fsp we already opened. Set up the + * base_fsp pointer. + */ + fsp->base_fsp = base_fsp; + } + /* * If it's a request for a directory open, deal with it separately. */ @@ -5767,45 +5762,19 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, status = open_directory(conn, req, dirfsp, - smb_fname, access_mask, share_access, create_disposition, create_options, file_attributes, &info, - &fsp); + fsp); } else { /* * Ordinary file case. */ - status = file_new(req, conn, &fsp); - if(!NT_STATUS_IS_OK(status)) { - goto fail; - } - - status = fsp_set_smb_fname(fsp, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - - if (*dirfsp == fsp->conn->cwd_fsp) { - fsp->dirfsp = fsp->conn->cwd_fsp; - } else { - fsp->dirfsp = talloc_move(fsp, dirfsp); - } - - if (base_fsp) { - /* - * We're opening the stream element of a - * base_fsp we already opened. Set up the - * base_fsp pointer. - */ - fsp->base_fsp = base_fsp; - } - if (allocation_size) { fsp->initial_allocation_size = smb_roundup(fsp->conn, allocation_size); @@ -5823,12 +5792,6 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, private_flags, &info, fsp); - - if(!NT_STATUS_IS_OK(status)) { - file_free(req, fsp); - fsp = NULL; - } - if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_IS_A_DIRECTORY)) { /* A stream open never opens a directory */ @@ -5852,18 +5815,19 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, status = open_directory(conn, -- Samba Shared Repository