On Thu, Mar 23, 2017 at 11:30:09AM +0100, Mathieu Parent wrote: > Hi, > > Today samba has released a security fix for a symlink race (leading to > information disclosure). > > Salvatore will take care of the jessie upload, I have uploaded for > sid, but we have not done anything on the wheezy side. > > See attached the backported patches for 3.6 (those are from the samba > bugzilla which is still embargoed). > > Please take care of it. >
Hello all, I have been able to figure out the minimum changes to cherry pick from the v3-6-stable branch in upstream Git. The commits are: 8234c6a 629e302 0a3b024 bc3714f d302cb6 94f7d0c 33ead72 66ee839 77cacee I was able to concatenate them into a single patch, which applied with only two offsets. After that the patch from upstream (3-6-racefix) applied with a bunch of small offsets. I have attached the consolidated and quilt-refreshed versions of both patches to this email. The patch containing the cherry picked commits which I have determined to be pre-requisites for upstream's patch is called 3-6-racefix-prereq.patch. The other patch file is the quilt-refreshed version of upstream's patch. Both of the attached patches apply cleanly to the 3.6.6-6+deb7u11 version of samba currently in wheezy. I have also built a 3.6.6-6+deb7u12 package with the two patches. The packages can be found here: https://people.debian.org/~roberto/ I still need to clean up the changelog entry. The packages could use some testing as well. I will try to do some testing, but give the scope of the changes (~850 lines of diff in total) more testing would certainly be a good thing. Also, I would appreciate any suggestions/feedback on minimizing the prereq patch. Regards, -Roberto -- Roberto C. Sánchez http://people.connexer.com/~roberto http://www.connexer.com
--- samba-3.6.6.orig/source3/smbd/dir.c
+++ samba-3.6.6/source3/smbd/dir.c
@@ -50,6 +50,8 @@
struct name_cache_entry *name_cache;
unsigned int name_cache_index;
unsigned int file_number;
+ files_struct *fsp; /* Back pointer to containing fsp, only
+ set from OpenDir_fsp(). */
};
struct dptr_struct {
@@ -1428,7 +1430,9 @@
if (fsp->is_directory && fsp->fh->fd != -1) {
dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr);
- if (dirp->dir == NULL) {
+ if (dirp->dir != NULL) {
+ dirp->fsp = fsp;
+ } else {
DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned "
"NULL (%s)\n",
dirp->dir_path,
--- samba-3.6.6.orig/source3/modules/vfs_dirsort.c
+++ samba-3.6.6/source3/modules/vfs_dirsort.c
@@ -30,40 +30,60 @@
struct dirsort_privates {
long pos;
SMB_STRUCT_DIRENT *directory_list;
- long number_of_entries;
- time_t mtime;
+ unsigned int number_of_entries;
+ struct timespec mtime;
SMB_STRUCT_DIR *source_directory;
- int fd;
+ files_struct *fsp; /* If open via FDOPENDIR. */
+ struct smb_filename *smb_fname; /* If open via OPENDIR */
};
static void free_dirsort_privates(void **datap) {
- struct dirsort_privates *data = (struct dirsort_privates *) *datap;
- SAFE_FREE(data->directory_list);
- SAFE_FREE(data);
- *datap = NULL;
-
- return;
+ TALLOC_FREE(*datap);
}
-static bool open_and_sort_dir (vfs_handle_struct *handle)
+static bool get_sorted_dir_mtime(vfs_handle_struct *handle,
+ struct dirsort_privates *data,
+ struct timespec *ret_mtime)
{
- SMB_STRUCT_DIRENT *dp;
- struct stat dir_stat;
- long current_pos;
- struct dirsort_privates *data = NULL;
+ int ret;
+ struct timespec mtime;
- SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates,
- return false);
+ if (data->fsp) {
+ ret = fsp_stat(data->fsp);
+ mtime = data->fsp->fsp_name->st.st_ex_mtime;
+ } else {
+ ret = SMB_VFS_STAT(handle->conn, data->smb_fname);
+ mtime = data->smb_fname->st.st_ex_mtime;
+ }
+
+ if (ret == -1) {
+ return false;
+ }
+
+ *ret_mtime = mtime;
+
+ return true;
+}
+
+static bool open_and_sort_dir(vfs_handle_struct *handle,
+ struct dirsort_privates *data)
+{
+ unsigned int i = 0;
+ unsigned int total_count = 0;
data->number_of_entries = 0;
- if (fstat(data->fd, &dir_stat) == 0) {
- data->mtime = dir_stat.st_mtime;
+ if (get_sorted_dir_mtime(handle, data, &data->mtime) == false) {
+ return false;
}
while (SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL)
!= NULL) {
- data->number_of_entries++;
+ total_count++;
+ }
+
+ if (total_count == 0) {
+ return false;
}
/* Open the underlying directory and count the number of entries
@@ -71,21 +91,26 @@
SMB_VFS_NEXT_REWINDDIR(handle, data->source_directory);
/* Set up an array and read the directory entries into it */
- SAFE_FREE(data->directory_list); /* destroy previous cache if needed */
- data->directory_list = (SMB_STRUCT_DIRENT *)SMB_MALLOC(
- data->number_of_entries * sizeof(SMB_STRUCT_DIRENT));
+ TALLOC_FREE(data->directory_list); /* destroy previous cache if needed */
+ data->directory_list = talloc_zero_array(data,
+ SMB_STRUCT_DIRENT,
+ total_count);
if (!data->directory_list) {
return false;
}
- current_pos = data->pos;
- data->pos = 0;
- while ((dp = SMB_VFS_NEXT_READDIR(handle, data->source_directory,
- NULL)) != NULL) {
- data->directory_list[data->pos++] = *dp;
+ for (i = 0; i < total_count; i++) {
+ SMB_STRUCT_DIRENT *dp = SMB_VFS_NEXT_READDIR(handle,
+ data->source_directory,
+ NULL);
+ if (dp == NULL) {
+ break;
+ }
+ data->directory_list[i] = *dp;
}
+ data->number_of_entries = i;
+
/* Sort the directory entries by name */
- data->pos = current_pos;
TYPESAFE_QSORT(data->directory_list, data->number_of_entries, compare_dirent);
return true;
}
@@ -94,12 +119,11 @@
const char *fname, const char *mask,
uint32 attr)
{
+ NTSTATUS status;
struct dirsort_privates *data = NULL;
/* set up our private data about this directory */
- data = (struct dirsort_privates *)SMB_MALLOC(
- sizeof(struct dirsort_privates));
-
+ data = talloc_zero(handle->conn, struct dirsort_privates);
if (!data) {
return NULL;
}
@@ -107,20 +131,34 @@
data->directory_list = NULL;
data->pos = 0;
+ status = create_synthetic_smb_fname(data,
+ fname,
+ NULL,
+ NULL,
+ &data->smb_fname);
+ if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(data);
+ return NULL;
+ }
+
/* Open the underlying directory and count the number of entries */
data->source_directory = SMB_VFS_NEXT_OPENDIR(handle, fname, mask,
attr);
- data->fd = dirfd(data->source_directory);
-
- SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
- struct dirsort_privates, return NULL);
+ if (data->source_directory == NULL) {
+ TALLOC_FREE(data);
+ return NULL;
+ }
- if (!open_and_sort_dir(handle)) {
+ if (!open_and_sort_dir(handle, data)) {
SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory);
+ TALLOC_FREE(data);
return NULL;
}
+ SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
+ struct dirsort_privates, return NULL);
+
return data->source_directory;
}
@@ -132,37 +170,35 @@
struct dirsort_privates *data = NULL;
/* set up our private data about this directory */
- data = (struct dirsort_privates *)SMB_MALLOC(
- sizeof(struct dirsort_privates));
-
+ data = talloc_zero(handle->conn, struct dirsort_privates);
if (!data) {
return NULL;
}
data->directory_list = NULL;
data->pos = 0;
+ data->fsp = fsp;
/* Open the underlying directory and count the number of entries */
data->source_directory = SMB_VFS_NEXT_FDOPENDIR(handle, fsp, mask,
attr);
if (data->source_directory == NULL) {
- SAFE_FREE(data);
+ TALLOC_FREE(data);
return NULL;
}
- data->fd = dirfd(data->source_directory);
-
- SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
- struct dirsort_privates, return NULL);
-
- if (!open_and_sort_dir(handle)) {
+ if (!open_and_sort_dir(handle, data)) {
SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory);
+ TALLOC_FREE(data);
/* fd is now closed. */
fsp->fh->fd = -1;
return NULL;
}
+ SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
+ struct dirsort_privates, return NULL);
+
return data->source_directory;
}
@@ -171,21 +207,18 @@
SMB_STRUCT_STAT *sbuf)
{
struct dirsort_privates *data = NULL;
- time_t current_mtime;
- struct stat dir_stat;
+ struct timespec current_mtime;
SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates,
return NULL);
- if (fstat(data->fd, &dir_stat) == -1) {
+ if (get_sorted_dir_mtime(handle, data, ¤t_mtime) == false) {
return NULL;
}
- current_mtime = dir_stat.st_mtime;
-
/* throw away cache and re-read the directory if we've changed */
- if (current_mtime > data->mtime) {
- open_and_sort_dir(handle);
+ if (timespec_compare(¤t_mtime, &data->mtime) > 1) {
+ open_and_sort_dir(handle, data);
}
if (data->pos >= data->number_of_entries) {
From ec1bca1d5315549e945c93cbf5e3abdb695de782 Mon Sep 17 00:00:00 2001 From: Jeremy Allison <j...@samba.org> Date: Mon, 20 Mar 2017 11:32:19 -0700 Subject: [PATCH 01/15] CVE-2017-2619: s3/smbd: re-open directory after dptr_CloseDir() dptr_CloseDir() will close and invalidate the fsp's file descriptor, we have to reopen it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Ralph Bohme <s...@samba.org> Signed-off-by: Jeremy Allison <j...@samba.org> --- source3/smbd/open.c | 2 +- source3/smbd/proto.h | 2 ++ source3/smbd/smb2_find.c | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) --- samba-3.6.6.orig/source3/smbd/open.c +++ samba-3.6.6/source3/smbd/open.c @@ -187,10 +187,277 @@ } /**************************************************************************** + Handle differing symlink errno's +****************************************************************************/ + +static int link_errno_convert(int err) +{ +#if defined(ENOTSUP) && defined(OSF1) + /* handle special Tru64 errno */ + if (err == ENOTSUP) { + err = ELOOP; + } +#endif /* ENOTSUP */ +#ifdef EFTYPE + /* fix broken NetBSD errno */ + if (err == EFTYPE) { + err = ELOOP; + } +#endif /* EFTYPE */ + /* fix broken FreeBSD errno */ + if (err == EMLINK) { + err = ELOOP; + } + return err; +} + +static int non_widelink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth); + +/**************************************************************************** + Follow a symlink in userspace. +****************************************************************************/ + +static int process_symlink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth) +{ + int fd = -1; + char *link_target = NULL; + int link_len = -1; + char *oldwd = NULL; + size_t rootdir_len = 0; + char *resolved_name = NULL; + bool matched = false; + int saved_errno = 0; + + /* + * Ensure we don't get stuck in a symlink loop. + */ + link_depth++; + if (link_depth >= 20) { + errno = ELOOP; + goto out; + } + + /* Allocate space for the link target. */ + link_target = talloc_array(talloc_tos(), char, PATH_MAX); + if (link_target == NULL) { + errno = ENOMEM; + goto out; + } + + /* Read the link target. */ + link_len = SMB_VFS_READLINK(conn, + smb_fname->base_name, + link_target, + PATH_MAX - 1); + if (link_len == -1) { + goto out; + } + + /* Ensure it's at least null terminated. */ + link_target[link_len] = '\0'; + + /* Convert to an absolute path. */ + resolved_name = SMB_VFS_REALPATH(conn, link_target); + if (resolved_name == NULL) { + goto out; + } + + /* + * We know conn_rootdir starts with '/' and + * does not end in '/'. FIXME ! Should we + * smb_assert this ? + */ + rootdir_len = strlen(conn_rootdir); + + matched = (strncmp(conn_rootdir, resolved_name, rootdir_len) == 0); + if (!matched) { + errno = EACCES; + goto out; + } + + /* + * Turn into a path relative to the share root. + */ + if (resolved_name[rootdir_len] == '\0') { + /* Link to the root of the share. */ + smb_fname->base_name = talloc_strdup(talloc_tos(), "."); + if (smb_fname->base_name == NULL) { + errno = ENOMEM; + goto out; + } + } else if (resolved_name[rootdir_len] == '/') { + smb_fname->base_name = &resolved_name[rootdir_len+1]; + } else { + errno = EACCES; + goto out; + } + + oldwd = vfs_GetWd(talloc_tos(), conn); + if (oldwd == NULL) { + goto out; + } + + /* Ensure we operate from the root of the share. */ + if (vfs_ChDir(conn, conn_rootdir) == -1) { + goto out; + } + + /* And do it all again.. */ + fd = non_widelink_open(conn, + conn_rootdir, + fsp, + smb_fname, + flags, + mode, + link_depth); + if (fd == -1) { + saved_errno = errno; + } + + out: + + SAFE_FREE(resolved_name); + TALLOC_FREE(link_target); + if (oldwd != NULL) { + int ret = vfs_ChDir(conn, oldwd); + if (ret == -1) { + smb_panic("unable to get back to old directory\n"); + } + TALLOC_FREE(oldwd); + } + if (saved_errno != 0) { + errno = saved_errno; + } + return fd; +} + +/**************************************************************************** + Non-widelink open. +****************************************************************************/ + +static int non_widelink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth) +{ + NTSTATUS status; + int fd = -1; + struct smb_filename *smb_fname_rel = NULL; + int saved_errno = 0; + char *oldwd = NULL; + char *parent_dir = NULL; + const char *final_component = NULL; + + if (!parent_dirname(talloc_tos(), + smb_fname->base_name, + &parent_dir, + &final_component)) { + goto out; + } + + oldwd = vfs_GetWd(talloc_tos(), conn); + if (oldwd == NULL) { + goto out; + } + + /* Pin parent directory in place. */ + if (vfs_ChDir(conn, parent_dir) == -1) { + goto out; + } + + /* Ensure the relative path is below the share. */ + status = check_reduced_name(conn, final_component); + if (!NT_STATUS_IS_OK(status)) { + saved_errno = map_errno_from_nt_status(status); + goto out; + } + + status = create_synthetic_smb_fname(talloc_tos(), + final_component, + smb_fname->stream_name, + &smb_fname->st, + &smb_fname_rel); + if (!NT_STATUS_IS_OK(status)) { + saved_errno = map_errno_from_nt_status(status); + goto out; + } + + flags |= O_NOFOLLOW; + + { + struct smb_filename *tmp_name = fsp->fsp_name; + fsp->fsp_name = smb_fname_rel; + fd = SMB_VFS_OPEN(conn, smb_fname_rel, fsp, flags, mode); + fsp->fsp_name = tmp_name; + } + + if (fd == -1) { + saved_errno = link_errno_convert(errno); + if (saved_errno == ELOOP) { + if (fsp->posix_open) { + /* Never follow symlinks on posix open. */ + goto out; + } + if (!lp_symlinks(SNUM(conn))) { + /* Explicitly no symlinks. */ + goto out; + } + /* + * We have a symlink. Follow in userspace + * to ensure it's under the share definition. + */ + fd = process_symlink_open(conn, + conn_rootdir, + fsp, + smb_fname_rel, + flags, + mode, + link_depth); + if (fd == -1) { + saved_errno = + link_errno_convert(errno); + } + } + } + + out: + + TALLOC_FREE(parent_dir); + TALLOC_FREE(smb_fname_rel); + + if (oldwd != NULL) { + int ret = vfs_ChDir(conn, oldwd); + if (ret == -1) { + smb_panic("unable to get back to old directory\n"); + } + TALLOC_FREE(oldwd); + } + if (saved_errno != 0) { + errno = saved_errno; + } + return fd; +} + +/**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ -static NTSTATUS fd_open(struct connection_struct *conn, +NTSTATUS fd_open(struct connection_struct *conn, files_struct *fsp, int flags, mode_t mode) @@ -198,8 +465,7 @@ struct smb_filename *smb_fname = fsp->fsp_name; NTSTATUS status = NT_STATUS_OK; -#ifdef O_NOFOLLOW - /* + /* * Never follow symlinks on a POSIX client. The * client should be doing this. */ @@ -207,12 +473,33 @@ if (fsp->posix_open || !lp_symlinks(SNUM(conn))) { flags |= O_NOFOLLOW; } -#endif - fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); + /* Ensure path is below share definition. */ + if (!lp_widelinks(SNUM(conn))) { + const char *conn_rootdir = SMB_VFS_CONNECTPATH(conn, + smb_fname->base_name); + if (conn_rootdir == NULL) { + return NT_STATUS_NO_MEMORY; + } + /* + * Only follow symlinks within a share + * definition. + */ + fsp->fh->fd = non_widelink_open(conn, + conn_rootdir, + fsp, + smb_fname, + flags, + mode, + 0); + } else { + fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); + } + if (fsp->fh->fd == -1) { - status = map_nt_error_from_unix(errno); - if (errno == EMFILE) { + int posix_errno = link_errno_convert(errno); + status = map_nt_error_from_unix(posix_errno); + if (posix_errno == EMFILE) { static time_t last_warned = 0L; if (time((time_t *) NULL) > last_warned) { --- samba-3.6.6.orig/source3/smbd/proto.h +++ samba-3.6.6/source3/smbd/proto.h @@ -592,6 +592,8 @@ const struct security_token *token, uint32_t access_desired, uint32_t *access_granted); +NTSTATUS fd_open(struct connection_struct *conn, files_struct *fsp, + int flags, mode_t mode); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, --- samba-3.6.6.orig/source3/smbd/smb2_find.c +++ samba-3.6.6/source3/smbd/smb2_find.c @@ -24,6 +24,7 @@ #include "../libcli/smb/smb_common.h" #include "trans2.h" #include "../lib/util/tevent_ntstatus.h" +#include "system/filesys.h" static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -300,7 +301,23 @@ } if (in_flags & SMB2_CONTINUE_FLAG_REOPEN) { + int flags; + dptr_CloseDir(fsp); + + /* + * dptr_CloseDir() will close and invalidate the fsp's file + * descriptor, we have to reopen it. + */ + + flags = O_RDONLY; +#ifdef O_DIRECTORY + flags |= O_DIRECTORY; +#endif + status = fd_open(conn, fsp, flags, 0); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } } if (fsp->dptr == NULL) { --- samba-3.6.6.orig/source3/modules/vfs_dirsort.c +++ samba-3.6.6/source3/modules/vfs_dirsort.c @@ -141,6 +141,10 @@ return NULL; } + if (ISDOT(data->smb_fname->base_name)) { + data->smb_fname->base_name = vfs_GetWd(data, handle->conn); + } + /* Open the underlying directory and count the number of entries */ data->source_directory = SMB_VFS_NEXT_OPENDIR(handle, fname, mask, attr); --- samba-3.6.6.orig/source3/modules/vfs_streams_xattr.c +++ samba-3.6.6/source3/modules/vfs_streams_xattr.c @@ -229,7 +229,7 @@ return -1; } - sbuf->st_ex_size = get_xattr_size(handle->conn, fsp->base_fsp, + sbuf->st_ex_size = get_xattr_size(handle->conn, fsp, io->base, io->xattr_name); if (sbuf->st_ex_size == -1) { return -1; @@ -364,6 +364,7 @@ char *xattr_name = NULL; int baseflags; int hostfd = -1; + int ret; DEBUG(10, ("streams_xattr_open called for %s\n", smb_fname_str_dbg(smb_fname))); @@ -375,133 +376,125 @@ /* If the default stream is requested, just open the base file. */ if (is_ntfs_default_stream_smb_fname(smb_fname)) { char *tmp_stream_name; - int ret; tmp_stream_name = smb_fname->stream_name; smb_fname->stream_name = NULL; ret = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); - smb_fname->stream_name = tmp_stream_name; - - return ret; - } + smb_fname->stream_name = tmp_stream_name; - status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name, - &xattr_name); - if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); - goto fail; - } + return ret; + } - /* Create an smb_filename with stream_name == NULL. */ - status = create_synthetic_smb_fname(talloc_tos(), - smb_fname->base_name, - NULL, NULL, - &smb_fname_base); - if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); - goto fail; - } + status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name, + &xattr_name); + if (!NT_STATUS_IS_OK(status)) { + errno = map_errno_from_nt_status(status); + goto fail; + } - /* - * We use baseflags to turn off nasty side-effects when opening the - * underlying file. - */ - baseflags = flags; - baseflags &= ~O_TRUNC; - baseflags &= ~O_EXCL; - baseflags &= ~O_CREAT; - - hostfd = SMB_VFS_OPEN(handle->conn, smb_fname_base, fsp, - baseflags, mode); - - TALLOC_FREE(smb_fname_base); - - /* It is legit to open a stream on a directory, but the base - * fd has to be read-only. - */ - if ((hostfd == -1) && (errno == EISDIR)) { - baseflags &= ~O_ACCMODE; - baseflags |= O_RDONLY; - hostfd = SMB_VFS_OPEN(handle->conn, smb_fname, fsp, baseflags, - mode); - } + /* Create an smb_filename with stream_name == NULL. */ + status = create_synthetic_smb_fname(talloc_tos(), + smb_fname->base_name, + NULL, NULL, + &smb_fname_base); + if (!NT_STATUS_IS_OK(status)) { + errno = map_errno_from_nt_status(status); + goto fail; + } - if (hostfd == -1) { - goto fail; - } + /* + * We use baseflags to turn off nasty side-effects when opening the + * underlying file. + */ + baseflags = flags; + baseflags &= ~O_TRUNC; + baseflags &= ~O_EXCL; + baseflags &= ~O_CREAT; - status = get_ea_value(talloc_tos(), handle->conn, NULL, - smb_fname->base_name, xattr_name, &ea); + hostfd = SMB_VFS_OPEN(handle->conn, smb_fname_base, fsp, + baseflags, mode); - DEBUG(10, ("get_ea_value returned %s\n", nt_errstr(status))); + TALLOC_FREE(smb_fname_base); - if (!NT_STATUS_IS_OK(status) - && !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { - /* - * The base file is not there. This is an error even if we got - * O_CREAT, the higher levels should have created the base - * file for us. + /* It is legit to open a stream on a directory, but the base + * fd has to be read-only. */ - DEBUG(10, ("streams_xattr_open: base file %s not around, " - "returning ENOENT\n", smb_fname->base_name)); - errno = ENOENT; - goto fail; - } + if ((hostfd == -1) && (errno == EISDIR)) { + baseflags &= ~O_ACCMODE; + baseflags |= O_RDONLY; + hostfd = SMB_VFS_OPEN(handle->conn, smb_fname, fsp, baseflags, + mode); + } - if (!NT_STATUS_IS_OK(status)) { - /* - * The attribute does not exist - */ + if (hostfd == -1) { + goto fail; + } + + status = get_ea_value(talloc_tos(), handle->conn, NULL, + smb_fname->base_name, xattr_name, &ea); - if (flags & O_CREAT) { + DEBUG(10, ("get_ea_value returned %s\n", nt_errstr(status))); + + if (!NT_STATUS_IS_OK(status) + && !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { /* - * Darn, xattrs need at least 1 byte + * The base file is not there. This is an error even if we got + * O_CREAT, the higher levels should have created the base + * file for us. */ - char null = '\0'; + DEBUG(10, ("streams_xattr_open: base file %s not around, " + "returning ENOENT\n", smb_fname->base_name)); + errno = ENOENT; + goto fail; + } - DEBUG(10, ("creating attribute %s on file %s\n", - xattr_name, smb_fname->base_name)); + if (!NT_STATUS_IS_OK(status)) { + /* + * The attribute does not exist + */ + if (flags & O_CREAT) { + /* + * Darn, xattrs need at least 1 byte + */ + char null = '\0'; + + DEBUG(10, ("creating attribute %s on file %s\n", + xattr_name, smb_fname->base_name)); + + fsp->fh->fd = hostfd; + ret = SMB_VFS_FSETXATTR(fsp, xattr_name, + &null, sizeof(null), + flags & O_EXCL ? XATTR_CREATE : 0); + fsp->fh->fd = -1; + if (ret != 0) { + goto fail; + } + } + } + + if (flags & O_TRUNC) { + char null = '\0'; if (fsp->base_fsp->fh->fd != -1) { - if (SMB_VFS_FSETXATTR( - fsp->base_fsp, xattr_name, - &null, sizeof(null), - flags & O_EXCL ? XATTR_CREATE : 0) == -1) { + if (SMB_VFS_FSETXATTR( + fsp->base_fsp, xattr_name, + &null, sizeof(null), + flags & O_EXCL ? XATTR_CREATE : 0) == -1) { goto fail; } } else { - if (SMB_VFS_SETXATTR( - handle->conn, smb_fname->base_name, - xattr_name, &null, sizeof(null), - flags & O_EXCL ? XATTR_CREATE : 0) == -1) { + if (SMB_VFS_SETXATTR( + handle->conn, smb_fname->base_name, + xattr_name, &null, sizeof(null), + flags & O_EXCL ? XATTR_CREATE : 0) == -1) { goto fail; } } } - } - - if (flags & O_TRUNC) { - char null = '\0'; - if (fsp->base_fsp->fh->fd != -1) { - if (SMB_VFS_FSETXATTR( - fsp->base_fsp, xattr_name, - &null, sizeof(null), - flags & O_EXCL ? XATTR_CREATE : 0) == -1) { - goto fail; - } - } else { - if (SMB_VFS_SETXATTR( - handle->conn, smb_fname->base_name, - xattr_name, &null, sizeof(null), - flags & O_EXCL ? XATTR_CREATE : 0) == -1) { - goto fail; - } - } - } - sio = (struct stream_io *)VFS_ADD_FSP_EXTENSION(handle, fsp, + sio = (struct stream_io *)VFS_ADD_FSP_EXTENSION(handle, fsp, struct stream_io, NULL); if (sio == NULL) { @@ -511,8 +504,15 @@ sio->xattr_name = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(handle, fsp), xattr_name); + /* + * sio->base needs to be a copy of fsp->fsp_name->base_name, + * making it identical to streams_xattr_recheck(). If the + * open is changing directories, fsp->fsp_name->base_name + * will be the full path from the share root, whilst + * smb_fname will be relative to the $cwd. + */ sio->base = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(handle, fsp), - smb_fname->base_name); + fsp->fsp_name->base_name); sio->fsp_name_ptr = fsp->fsp_name; sio->handle = handle; sio->fsp = fsp; @@ -861,7 +861,7 @@ return -1; } - status = get_ea_value(talloc_tos(), handle->conn, fsp->base_fsp, + status = get_ea_value(talloc_tos(), handle->conn, fsp, sio->base, sio->xattr_name, &ea); if (!NT_STATUS_IS_OK(status)) { return -1; @@ -885,13 +885,13 @@ memcpy(ea.value.data + offset, data, n); - if (fsp->base_fsp->fh->fd != -1) { - ret = SMB_VFS_FSETXATTR(fsp->base_fsp, + if (fsp->fh->fd != -1) { + ret = SMB_VFS_FSETXATTR(fsp, sio->xattr_name, ea.value.data, ea.value.length, 0); } else { ret = SMB_VFS_SETXATTR(fsp->conn, - fsp->base_fsp->fsp_name->base_name, + fsp->fsp_name->base_name, sio->xattr_name, ea.value.data, ea.value.length, 0); } @@ -925,7 +925,7 @@ return -1; } - status = get_ea_value(talloc_tos(), handle->conn, fsp->base_fsp, + status = get_ea_value(talloc_tos(), handle->conn, fsp, sio->base, sio->xattr_name, &ea); if (!NT_STATUS_IS_OK(status)) { return -1; @@ -970,7 +970,7 @@ return -1; } - status = get_ea_value(talloc_tos(), handle->conn, fsp->base_fsp, + status = get_ea_value(talloc_tos(), handle->conn, fsp, sio->base, sio->xattr_name, &ea); if (!NT_STATUS_IS_OK(status)) { return -1; @@ -995,13 +995,13 @@ ea.value.length = offset + 1; ea.value.data[offset] = 0; - if (fsp->base_fsp->fh->fd != -1) { - ret = SMB_VFS_FSETXATTR(fsp->base_fsp, + if (fsp->fh->fd != -1) { + ret = SMB_VFS_FSETXATTR(fsp, sio->xattr_name, ea.value.data, ea.value.length, 0); } else { ret = SMB_VFS_SETXATTR(fsp->conn, - fsp->base_fsp->fsp_name->base_name, + fsp->fsp_name->base_name, sio->xattr_name, ea.value.data, ea.value.length, 0); } --- samba-3.6.6.orig/source3/smbd/dir.c +++ samba-3.6.6/source3/smbd/dir.c @@ -1358,7 +1358,8 @@ Open a directory. ********************************************************************/ -struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, +static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, + connection_struct *conn, const char *name, const char *mask, uint32 attr) @@ -1370,27 +1371,21 @@ return NULL; } - dirp->conn = conn; - dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn)); - - dirp->dir_path = talloc_strdup(dirp, name); - if (!dirp->dir_path) { - errno = ENOMEM; + dirp->dir = SMB_VFS_OPENDIR(conn, name, mask, attr); + if (!dirp->dir) { + DEBUG(5,("OpenDir: Can't open %s. %s\n", name, + strerror(errno) )); goto fail; } + dirp->conn = conn; + dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn)); + if (sconn && !sconn->using_smb2) { sconn->searches.dirhandles_open++; } talloc_set_destructor(dirp, smb_Dir_destructor); - dirp->dir = SMB_VFS_OPENDIR(conn, dirp->dir_path, mask, attr); - if (!dirp->dir) { - DEBUG(5,("OpenDir: Can't open %s. %s\n", dirp->dir_path, - strerror(errno) )); - goto fail; - } - return dirp; fail: @@ -1398,6 +1393,76 @@ return NULL; } +/**************************************************************************** + Open a directory handle by pathname, ensuring it's under the share path. +****************************************************************************/ + +static struct smb_Dir *open_dir_safely(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name, + const char *wcard, + uint32_t attr) +{ + struct smb_Dir *dir_hnd = NULL; + char *saved_dir = vfs_GetWd(ctx, conn); + NTSTATUS status; + + if (saved_dir == NULL) { + return NULL; + } + + if (vfs_ChDir(conn, name) == -1) { + goto out; + } + + /* + * Now the directory is pinned, use + * REALPATH to ensure we can access it. + */ + status = check_name(conn, "."); + if (!NT_STATUS_IS_OK(status)) { + goto out; + } + + dir_hnd = OpenDir_internal(ctx, + conn, + ".", + wcard, + attr); + + if (dir_hnd == NULL) { + goto out; + } + + /* + * OpenDir_internal only gets "." as the dir name. + * Store the real dir name here. + */ + + dir_hnd->dir_path = talloc_strdup(dir_hnd, name); + if (!dir_hnd->dir_path) { + errno = ENOMEM; + } + + out: + + vfs_ChDir(conn, saved_dir); + TALLOC_FREE(saved_dir); + return dir_hnd; +} + +struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, + const char *name, + const char *mask, + uint32_t attr) +{ + return open_dir_safely(mem_ctx, + conn, + name, + mask, + attr); +} + /******************************************************************* Open a directory from an fsp. ********************************************************************/ @@ -1411,7 +1476,17 @@ struct smbd_server_connection *sconn = conn->sconn; if (!dirp) { - return NULL; + goto fail; + } + + if (!fsp->is_directory) { + errno = EBADF; + goto fail; + } + + if (fsp->fh->fd == -1) { + errno = EBADF; + goto fail; } dirp->conn = conn; @@ -1423,36 +1498,33 @@ goto fail; } - if (sconn && !sconn->using_smb2) { - sconn->searches.dirhandles_open++; - } - talloc_set_destructor(dirp, smb_Dir_destructor); - - if (fsp->is_directory && fsp->fh->fd != -1) { - dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr); - if (dirp->dir != NULL) { - dirp->fsp = fsp; - } else { - DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned " - "NULL (%s)\n", - dirp->dir_path, - strerror(errno))); - if (errno != ENOSYS) { - return NULL; - } + dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr); + if (dirp->dir != NULL) { + dirp->fsp = fsp; + } else { + DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned " + "NULL (%s)\n", + dirp->dir_path, + strerror(errno))); + if (errno != ENOSYS) { + goto fail; } } if (dirp->dir == NULL) { - /* FDOPENDIR didn't work. Use OPENDIR instead. */ - dirp->dir = SMB_VFS_OPENDIR(conn, dirp->dir_path, mask, attr); + /* FDOPENDIR is not supported. Use OPENDIR instead. */ + TALLOC_FREE(dirp); + return open_dir_safely(mem_ctx, + conn, + fsp->fsp_name->base_name, + mask, + attr); } - if (!dirp->dir) { - DEBUG(5,("OpenDir_fsp: Can't open %s. %s\n", dirp->dir_path, - strerror(errno) )); - goto fail; + if (sconn && !sconn->using_smb2) { + sconn->searches.dirhandles_open++; } + talloc_set_destructor(dirp, smb_Dir_destructor); return dirp;
signature.asc
Description: Digital signature
