[PATCH] Fix bug #10406 - vfs_dirsort can trample on its own private data.
Jeremy Allison
jra at samba.org
Tue Feb 11 11:43:50 MST 2014
On Tue, Feb 11, 2014 at 11:14:58PM +1300, Andrew Bartlett wrote:
> On Mon, 2014-02-10 at 21:31 -0800, Jeremy Allison wrote:
> > Now tested and confirmed good by the OEM who reported it,
> > this patch fixes the problem that the current dirsort
> > module only keeps one active private data pointer on
> > the connection struct, so opening a second directory
> > on the same connection will overwrite it.
> >
> > Please review & push if happy !
>
> Can you make sure that in 'make test' we now run raw.search and the os2
> delete test against the tmpenc share (or perhaps better a new share for
> this purpose) that uses dirsort? It would be great to ensure this is
> tested.
New, washes whiter ! Cleans with more enzyme action ! :-).
make test now included :-).
Please review and push if happy !
Jeremy.
-------------- next part --------------
From 5269efea7d82d4f56b94713d70bf18aa5f669909 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jan 2014 17:01:30 -0800
Subject: [PATCH 1/2] s3: vfs_dirsort module.
Allow dirsort to work when multiple simultaneous
directories are open. The old code only keeps one
active private data pointer on the connection struct, opening
a second directory on the same connection will overwrite it.
This modification turns the private data pointer
into a linked list of open directories on the
connection struct, and finds the correct one by searching
on the passed in DIR *.
With this code in place, smbd passes raw.search
torture test on a share definition with:
vfs objects = dirsort
https://bugzilla.samba.org/show_bug.cgi?id=10406
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/modules/vfs_dirsort.c | 119 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 112 insertions(+), 7 deletions(-)
diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c
index 2c25765..98109c2 100644
--- a/source3/modules/vfs_dirsort.c
+++ b/source3/modules/vfs_dirsort.c
@@ -28,6 +28,7 @@ static int compare_dirent (const struct dirent *da, const struct dirent *db)
}
struct dirsort_privates {
+ struct dirsort_privates *prev, *next;
long pos;
struct dirent *directory_list;
unsigned int number_of_entries;
@@ -37,10 +38,6 @@ struct dirsort_privates {
struct smb_filename *smb_fname; /* If open via OPENDIR */
};
-static void free_dirsort_privates(void **datap) {
- TALLOC_FREE(*datap);
-}
-
static bool get_sorted_dir_mtime(vfs_handle_struct *handle,
struct dirsort_privates *data,
struct timespec *ret_mtime)
@@ -119,8 +116,15 @@ static DIR *dirsort_opendir(vfs_handle_struct *handle,
const char *fname, const char *mask,
uint32 attr)
{
+ struct dirsort_privates *list_head = NULL;
struct dirsort_privates *data = NULL;
+ if (SMB_VFS_HANDLE_TEST_DATA(handle)) {
+ /* Find the list head of all open directories. */
+ SMB_VFS_HANDLE_GET_DATA(handle, list_head, struct dirsort_privates,
+ return NULL);
+ }
+
/* set up our private data about this directory */
data = talloc_zero(handle->conn, struct dirsort_privates);
if (!data) {
@@ -148,7 +152,9 @@ static DIR *dirsort_opendir(vfs_handle_struct *handle,
return NULL;
}
- SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
+ /* Add to the private list of all open directories. */
+ DLIST_ADD(list_head, data);
+ SMB_VFS_HANDLE_SET_DATA(handle, list_head, NULL,
struct dirsort_privates, return NULL);
return data->source_directory;
@@ -159,8 +165,15 @@ static DIR *dirsort_fdopendir(vfs_handle_struct *handle,
const char *mask,
uint32 attr)
{
+ struct dirsort_privates *list_head = NULL;
struct dirsort_privates *data = NULL;
+ if (SMB_VFS_HANDLE_TEST_DATA(handle)) {
+ /* Find the list head of all open directories. */
+ SMB_VFS_HANDLE_GET_DATA(handle, list_head, struct dirsort_privates,
+ return NULL);
+ }
+
/* set up our private data about this directory */
data = talloc_zero(handle->conn, struct dirsort_privates);
if (!data) {
@@ -186,7 +199,9 @@ static DIR *dirsort_fdopendir(vfs_handle_struct *handle,
return NULL;
}
- SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates,
+ /* Add to the private list of all open directories. */
+ DLIST_ADD(list_head, data);
+ SMB_VFS_HANDLE_SET_DATA(handle, list_head, NULL,
struct dirsort_privates, return NULL);
return data->source_directory;
@@ -202,12 +217,20 @@ static struct dirent *dirsort_readdir(vfs_handle_struct *handle,
SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates,
return NULL);
+ while(data && (data->source_directory != dirp)) {
+ data = data->next;
+ }
+ if (data == NULL) {
+ return NULL;
+ }
+
if (get_sorted_dir_mtime(handle, data, ¤t_mtime) == false) {
return NULL;
}
/* throw away cache and re-read the directory if we've changed */
- if (timespec_compare(¤t_mtime, &data->mtime) > 1) {
+ if (timespec_compare(¤t_mtime, &data->mtime)) {
+ SMB_VFS_NEXT_REWINDDIR(handle, data->source_directory);
open_and_sort_dir(handle, data);
}
@@ -221,10 +244,53 @@ static struct dirent *dirsort_readdir(vfs_handle_struct *handle,
static void dirsort_seekdir(vfs_handle_struct *handle, DIR *dirp,
long offset)
{
+ struct timespec current_mtime;
struct dirsort_privates *data = NULL;
+
SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, return);
+ /* Find the entry holding dirp. */
+ while(data && (data->source_directory != dirp)) {
+ data = data->next;
+ }
+ if (data == NULL) {
+ return;
+ }
+ if (offset > data->number_of_entries) {
+ return;
+ }
data->pos = offset;
+
+ if (get_sorted_dir_mtime(handle, data, ¤t_mtime) == false) {
+ return;
+ }
+
+ if (timespec_compare(¤t_mtime, &data->mtime)) {
+ /* Directory changed. We must re-read the
+ cache and search for the name that was
+ previously stored at the offset being
+ requested, otherwise after the re-sort
+ we will point to the wrong entry. The
+ OS/2 incremental delete code relies on
+ this. */
+ unsigned int i;
+ char *wanted_name = talloc_strdup(handle->conn,
+ data->directory_list[offset].d_name);
+ if (wanted_name == NULL) {
+ return;
+ }
+ SMB_VFS_NEXT_REWINDDIR(handle, data->source_directory);
+ open_and_sort_dir(handle, data);
+ /* Now search for where we were. */
+ data->pos = 0;
+ for (i = 0; i < data->number_of_entries; i++) {
+ if(strcmp(wanted_name, data->directory_list[i].d_name) == 0) {
+ data->pos = i;
+ break;
+ }
+ }
+ TALLOC_FREE(wanted_name);
+ }
}
static long dirsort_telldir(vfs_handle_struct *handle, DIR *dirp)
@@ -233,6 +299,13 @@ static long dirsort_telldir(vfs_handle_struct *handle, DIR *dirp)
SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates,
return -1);
+ /* Find the entry holding dirp. */
+ while(data && (data->source_directory != dirp)) {
+ data = data->next;
+ }
+ if (data == NULL) {
+ return -1;
+ }
return data->pos;
}
@@ -241,9 +314,40 @@ static void dirsort_rewinddir(vfs_handle_struct *handle, DIR *dirp)
struct dirsort_privates *data = NULL;
SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, return);
+ /* Find the entry holding dirp. */
+ while(data && (data->source_directory != dirp)) {
+ data = data->next;
+ }
+ if (data == NULL) {
+ return;
+ }
data->pos = 0;
}
+static int dirsort_closedir(vfs_handle_struct *handle, DIR *dirp)
+{
+ struct dirsort_privates *list_head = NULL;
+ struct dirsort_privates *data = NULL;
+ int ret;
+
+ SMB_VFS_HANDLE_GET_DATA(handle, list_head, struct dirsort_privates, return -1);
+ /* Find the entry holding dirp. */
+ for(data = list_head; data && (data->source_directory != dirp); data = data->next) {
+ ;
+ }
+ if (data == NULL) {
+ return -1;
+ }
+ /* Remove from the list and re-store the list head. */
+ DLIST_REMOVE(list_head, data);
+ SMB_VFS_HANDLE_SET_DATA(handle, list_head, NULL,
+ struct dirsort_privates, return -1);
+
+ ret = SMB_VFS_NEXT_CLOSEDIR(handle, dirp);
+ TALLOC_FREE(data);
+ return ret;
+}
+
static struct vfs_fn_pointers vfs_dirsort_fns = {
.opendir_fn = dirsort_opendir,
.fdopendir_fn = dirsort_fdopendir,
@@ -251,6 +355,7 @@ static struct vfs_fn_pointers vfs_dirsort_fns = {
.seekdir_fn = dirsort_seekdir,
.telldir_fn = dirsort_telldir,
.rewind_dir_fn = dirsort_rewinddir,
+ .closedir_fn = dirsort_closedir,
};
NTSTATUS vfs_dirsort_init(void)
--
1.9.0.rc1.175.g0b1dcb5
From d132e8f33caf3cb260393f5e96cd5863afb6e73e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Feb 2014 10:39:04 -0800
Subject: [PATCH 2/2] s3: vfs_dirsort module.
Add raw.search torture test on a share definition with:
vfs objects = dirsort
https://bugzilla.samba.org/show_bug.cgi?id=10406
Signed-off-by: Jeremy Allison <jra at samba.org>
---
selftest/target/Samba3.pm | 4 ++++
source3/selftest/tests.py | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index cd1585e..d7b5177 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1066,6 +1066,10 @@ sub provision($$$$$$)
[tmp]
path = $shrdir
comment = smb username is [%U]
+[tmpsort]
+ path = $shrdir
+ comment = Load dirsort module
+ vfs objects = dirsort acl_xattr fake_acls xattr_tdb streams_depot
[tmpenc]
path = $shrdir
comment = encrypt smb username is [%U]
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 4ecd9c6..a2be3e4 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -361,6 +361,11 @@ for t in tests:
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
+ elif t == "raw.search":
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+# test the dirsort module.
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmpsort -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
else:
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
--
1.9.0.rc1.175.g0b1dcb5
More information about the samba-technical
mailing list